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

["Patrick Donnelly" <batrick.donnelly () gmail com>]

On Sun, Oct 12, 2008 at 7:56 AM, Sven Klemm <sven () c3d2 de> wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> David Fifield wrote:
> > 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.
>
> You got this error because ripeQuery.nse did not check the return
> value of  socket:connect(). The problem is resolved as ripeQuery now
> uses the comm library which properly checks the return values.
>
> Cheers,
> Sven

The real issue here is that something is causing a segmentation fault
(large problem). The script's method of achieving this is incidental.
The cause of this segmentation fault still needs to be addressed.



-- 
-Patrick Donnelly

"One of the lessons of history is that nothing is often a good thing
to do and always a clever thing to say."

-Will Durant

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