Re: Ndiff Bug found and fixed

[David Fifield <david () bamsoftware com>]

On Thu, Jan 21, 2010 at 05:59:59PM -0500, dlucci () purdue edu wrote:
> My name is Derril Lucci.  I am a Senior in Computer Science at Purdue
> University.  I found a bug in ndiff.  The bug is that it crashes if a
> file does not exist.  It occurs at in the Scan.load_from_file method.
> I have included the corrected code. 
>
> def load_fro_file(self, filename):
>     ""Load a scan from the Nmap XML file with the given filename.""
>     try:
>        f = open(filename, "r")
>        self.load(f)
>        f.close()
>     except IOError:
>        print u'Could not open', filename, u'.'
>        sys.exit(1)

Hi Derril. Thank you for writing. I can see your point about having a
nicer error message when a file doesn't exist. This is what it looks
like now:

$ ndiff bad bad
Traceback (most recent call last):
  File "/usr/bin/ndiff", line 1221, in <module>
    sys.exit(main())
  File "/usr/bin/ndiff", line 1197, in main
    scan_a.load_from_file(filename_a)
  File "/usr/bin/ndiff", line 54, in load_from_file
    f = open(filename, "r")
IOError: [Errno 2] No such file or directory: 'bad'

That's kind of ugly, but the "No such file or directory" is important to
keep (and not just replace with a generic "Could not open"). For
example, there are other ways to fail to open a file:

$ ndiff /root/1.xml /root/2.xml
Traceback (most recent call last):
  File "/usr/bin/ndiff", line 1221, in <module>
    sys.exit(main())
  File "/usr/bin/ndiff", line 1197, in main
    scan_a.load_from_file(filename_a)
  File "/usr/bin/ndiff", line 54, in load_from_file
    f = open(filename, "r")
IOError: [Errno 13] Permission denied: '/root/1.xml'

We can lose the stack traceback but we definitely want to keep Python's
exception message. I have just committed this:

    try:
        scan_a = Scan()
        scan_a.load_from_file(filename_a)
        scan_b = Scan()
        scan_b.load_from_file(filename_b)
    except Exception, e:
        print >> sys.stderr, u"Can't open file: %s" % str(e)
        sys.exit(EXIT_ERROR)

I think it's better to handle the error at a higher level than within
Scan.load_from_file. The only thing I'm not quite sure about is whether
this handles non-ASCII exceptions strings properly. In my tests it works
okay when a filename has a non-ASCII character, but I'm not sure it
works when the equivalent of "No such file or directory" has such
characters.

David Fifield
_______________________________________________
Sent through the nmap-dev mailing list
http://cgi.insecure.org/mailman/listinfo/nmap-dev
Archived at http://seclists.org/nmap-dev/