Re: [NSE][PATCH] throw an error in bin.pack for illegal hex characters

[David Fifield <david () bamsoftware com>]

On Fri, Oct 10, 2008 at 10:58:36AM -0600, Patrick Donnelly wrote:
> On Fri, Oct 10, 2008 at 9:50 AM, Sven Klemm <sven () c3d2 de> wrote:
> > currently bin.pack('H','ABCDEFG') does not throw an error for invalid
> > hex characters when packing hex data but ignores invalid input. The
> > attached patch changes this.
> > luaL_error only supports a very limited set of format specifiers
> > that's why I'm using %p for unprintable characters.
> >
> > SCRIPT ENGINE: ./scripts/debug.nse:30: Illegal hex character 'G'
> >
> > SCRIPT ENGINE: ./scripts/debug.nse:30: Illegal hex character 0xfa
> >
> > Cheers,
> > Sven
>
> It's probably best that this library's design decisions not be changed
> at this point. lpack is a fairly common library in the Lua community
> and people would not expect this to error. From just a quick search of
> the code, there appears to be no intentionally thrown errors by the
> library. It is also highly probable that large pieces of this library
> be integrated into a struct library in Lua 5.2, in which case we will
> most likely remove this library. In the interest of consistency, we
> should not change something unless it is truly warranted.

Patrick raises an excellent point. We should not introduce gratuitous
changes to our libraries that are based on external ones. In this case I
think that raising an error would be beneficial. It would not affect
correctly written programs and may help catch bugs. However, in this
particular situation it doesn't matter much either way.

This made me think of a similar problem I ran into before, which is
still unsolved as far as I know:

"Unhandled NSE exceptions"
http://seclists.org/nmap-dev/2007/q4/0203.html
"Re: Unhandled NSE exceptions"
http://seclists.org/nmap-dev/2008/q2/0614.html

I think it would be fixed if in this case nsock used an "error style"
rather than an "assert style," a distinction ably noted by Patrick.

David

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