Re: Completed Lua 5.3 upgrade!

[Paulino Calderon <paulino () calderonpale com>]

Hey Daniel,

I confirm your patch fixes the issue in my installation. I also agree that this change (r3600) is useful in the other 
places because spotting undefined behavior errors related the float-to-int conversion could be tricky. If we are 
dealing with signed values,  should we also use ceil() in nseU_checkinteger accordingly to get more accurate results?

Cheers
. 
> On Jul 17, 2016, at 12:02 AM, Daniel Miller <bonsaiviking () gmail com> wrote:
>
> Patrick,
>
> I think this is supposed to be handled by setting LUA_FLOORN2I, which causes a floor operation to be applied when 
> calling things like luaL_checkinteger on a float value. Unfortunately, we can't just set that because we'll end up 
> with these crashes if the user links against a liblua built without this set. So I think we need to go through and 
> apply a floor operation to most cases where we call luaL_checkinteger.
>
> Instead, I've pushed r36000, a change that adds a utility function nseU_checkinteger. This function works a bit like 
> luaL_checkinteger when LUA_FLOORN2I is set to 1: it checks that the given index is a number, then does a floor 
> operation on that number before calling lua_numbertointeger to convert it to an int (which is what lua_Integer is 
> probably set to in most cases). The other change involved is using signed integer timeouts instead of unsigned, since 
> the nsock operations all take signed values anyway. This will help avoid integer overflow problems.
>
> I think this function should be used in most places in the NSE libraries where we were previously using 
> luaL_checkinteger.  I also suspect it could be used for lua_tointeger in lots of places, too, but I would appreciate 
> your input.
>
> Dan
>
>
> On Fri, Jul 15, 2016 at 1:53 PM, Paulino Calderon <paulino () calderonpale com <mailto:paulino () calderonpale com>> 
> wrote:
> Hey,
>
> I agree with increasing the debug level required to at least 4. A lot of scripts use level 3 to print information and 
> those bin calls are useful but won’t be needed in a lot of cases as they produce a LOT of output.
>
> I just spotted another issue with the upgrade to 5.3. lua_tointeger changed behavior in 5.3 
> (https://www.lua.org/manual/5.3/manual.html#lua_tointeger <https://www.lua.org/manual/5.3/manual.html#lua_tointeger>) 
> and it is causing problems with scripts/libraries using explicitly non-specified conversions, specifically i noticed 
> the problem in stdnse.get_timeouts() and comm.setup_connect but I’m posting this to the mailing list in case we use 
> these integer conversions anywhere else:
>
> /Users/cldrn/Tools/nmap/./nselib/comm.lua:65: bad argument #1 to 'set_timeout' (number has no integer representation)
> stack traceback:
>         [C]: in method 'set_timeout'
>         /Users/cldrn/Tools/nmap/./nselib/comm.lua:65: in upvalue 'setup_connect'
>         /Users/cldrn/Tools/nmap/./nselib/comm.lua:211: in function 'comm.opencon'
>         /Users/cldrn/Tools/nmap/./nselib/comm.lua:263: in function 'comm.tryssl'
>         /Users/cldrn/Tools/nmap/./nselib/http.lua:1205: in function </Users/cldrn/Tools/nmap/./nselib/http.lua:1181>
>         (...tail calls...)
>         /Users/cldrn/Tools/nmap/./nselib/http.lua:1625: in function 'http.get'
>         /Users/cldrn/Tools/nmap/./nselib/http.lua:2479: in function 'http.identify_404'
>         /Users/cldrn/Tools/nmap/./scripts/http-enum.nse:370: in function 
> </Users/cldrn/Tools/nmap/./scripts/http-enum.nse:351>
>         (...tail calls...)
>
>
>
> Ps. I’ll post a patch to stdnse later today after I look for this problem in other parts of NSE.
>
> Cheers.
>
> > On Jul 7, 2016, at 5:45 PM, Patrick Donnelly <batrick () batbytes com <mailto:batrick () batbytes com>> wrote:
> >
> > On Thu, Jul 7, 2016 at 6:46 AM, Tom Sellers <nmap () fadedcode net <mailto:nmap () fadedcode net>> wrote:
> > > On 7/2/2016 12:06 PM, Patrick Donnelly wrote:
> > > > On Sun, Jun 26, 2016 at 4:53 PM, Patrick Donnelly <batrick () batbytes com <mailto:batrick () batbytes com>> 
> > > > wrote:
> > > > > On Sat, Jun 25, 2016 at 1:25 PM, Patrick Donnelly <batrick () batbytes com <mailto:batrick () batbytes com>> 
> > > > > wrote:
> > > > > > On Fri, Jun 17, 2016 at 8:43 PM, Patrick Donnelly <batrick () batbytes com <mailto:batrick () batbytes com>> 
> > > > > > wrote:
> > > > > > > On Wed, Jun 8, 2016 at 9:31 PM, Patrick Donnelly <batrick () batbytes com <mailto:batrick () batbytes com>> 
> > > > > > > wrote:
> > > > > > > > Without further ado, here's the branch:
> > > > > > > >
> > > > > > > > https://github.com/batrick/nmap/tree/nse-lua53 <https://github.com/batrick/nmap/tree/nse-lua53>
> > > > > > >
> > > > > > > If there are no questions/comments/complaints/objections, I'd like to
> > > > > > > merge the branch sometime around next Friday. All feedback is still
> > > > > > > welcome!
> > > > > >
> > > > > > I've incorporated Daniel's suggestions. The branch is now in svn:
> > > > > > https://svn.nmap.org/nmap-exp/patrick/nse-lua53 <https://svn.nmap.org/nmap-exp/patrick/nse-lua53>
> > > > > >
> > > > > > Last call for comments. I plan to merge into /nmap tomorrow.
> > > > >
> > > > > Alright, I've resolved all of the reported problems. I will give the
> > > > > branch another few days to cool before merging. Please keep testing
> > > > > everyone!
> > > >
> > > > Merged in https://svn.nmap.org/nmap@35945 <https://svn.nmap.org/nmap@35945>.
> > >
> > >
> > > Patrick,
> > >
> > >  Will the debug statements in bin.lua remain?  They generate the following at debug level2:
> >
> > I'm open to increasing the debug level. There may be bugs still in the
> > bin wrapper so I reckon the debug output may help in the future.
> >
> > --
> > Patrick Donnelly
> > _______________________________________________
> > Sent through the dev mailing list
> > https://nmap.org/mailman/listinfo/dev <https://nmap.org/mailman/listinfo/dev>
> > Archived at http://seclists.org/nmap-dev/ <http://seclists.org/nmap-dev/>
>
> _______________________________________________
> Sent through the dev mailing list
> https://nmap.org/mailman/listinfo/dev <https://nmap.org/mailman/listinfo/dev>
> Archived at http://seclists.org/nmap-dev/ <http://seclists.org/nmap-dev/>

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