Re: crash its not work hiks hikss...

[Daniel Miller <bonsaiviking () gmail com>]

On Wed, Apr 16, 2014 at 3:11 PM, Daniel Miller <bonsaiviking () gmail com>wrote:

> Here's another option, this time as a patch to Ndiff, which is similarly
> affected. The logic works the same, except this one doesn't care about
> finding a valid DTD, but instead returns an "empty file," short-circuiting
> the DTD parsing:
>
> diff --git a/ndiff/ndiff.py b/ndiff/ndiff.py
> index 28e99da..a8706f2 100755
> --- a/ndiff/ndiff.py
> +++ b/ndiff/ndiff.py
> @@ -21,12 +21,22 @@ import time
>  import xml.sax
>  import xml.sax.saxutils
>  import xml.dom.minidom
> +from StringIO import StringIO
>
>  verbose = False
>
>  NDIFF_XML_VERSION = u"1"
>
>
> +class OverrideEntityResolver(xml.sax.handler.EntityResolver):
>
> +    """This class overrides the default behavior of xml.sax to download
> +    remote DTDs, instead returning blank strings"""
> +    empty = StringIO()
> +
>
> +    def resolveEntity(self, publicId, systemId):
> +        return OverrideEntityResolver.empty
> +
> +
>  class Scan(object):
>      """A single Nmap scan, corresponding to a single invocation of Nmap.
> It is
>      a container for a list of hosts. It also has utility methods to load
> itself
> @@ -48,6 +58,7 @@ class Scan(object):
>          """Load a scan from the Nmap XML in the file-like object f."""
>          parser = xml.sax.make_parser()
>          handler = NmapContentHandler(self)
> +        parser.setEntityResolver(OverrideEntityResolver())
>          parser.setContentHandler(handler)
>          parser.parse(f)
>
>
> Of note: at least one Zenmap user has reported that reducing timing
> template from -T5 to -T4 prevented the error from occurring. This may be an
> indication that Nmap at -T5 is saturating some folks's network links. In
> any case, I don't think we should be making these parsers require Internet
> access to work.
>
> Dan

This seemed to be the cleanest way to do this, and on the plus side, ought
to protect Zenmap and Ndiff from XXE attacks, though I haven't thought
through the exact scenario that would cause it to be a problem. I added
this most recent method in r32833 and r32834 for Ndiff and Zenmap
respectively.

This does not answer the question of whether we want to continue putting
the DOCTYPE into Nmap's output, since older versions of Zenmap and Ndiff
will continue to exhibit this behavior when parsing new Nmap output.

Dan
_______________________________________________
Sent through the dev mailing list
http://nmap.org/mailman/listinfo/dev
Archived at http://seclists.org/nmap-dev/