illegal luaL_buffer usage in loadlib.c

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

illegal luaL_buffer usage in loadlib.c

Daurnimator
In findloader (https://www.lua.org/source/5.3/loadlib.c.html#findloader)
luaL_buffinit is called before pushing the searchers table onto the
stack.
This breaks the "balanced stack" that luaL_buffinit is documented to expect.
Also, later code then assumes that the searchers table is at index 3.

Now, this doesn't actually manifest in a bug in PUC Rio lua 5.3, as
the implementation of luaL_buffinit doesn't touch the stack.
However if someone were to extract parts of loadlib.c to use with a
different auxlib implementation then there might be issues.

The fix is to simply move the luaL_buffinit call below the lua_getfield call.

Reply | Threaded
Open this post in threaded view
|

Re: illegal luaL_buffer usage in loadlib.c

Roberto Ierusalimschy
> In findloader (https://www.lua.org/source/5.3/loadlib.c.html#findloader)
> luaL_buffinit is called before pushing the searchers table onto the
> stack.
> This breaks the "balanced stack" that luaL_buffinit is documented to expect.
> Also, later code then assumes that the searchers table is at index 3.
>
> Now, this doesn't actually manifest in a bug in PUC Rio lua 5.3, as
> the implementation of luaL_buffinit doesn't touch the stack.
> However if someone were to extract parts of loadlib.c to use with a
> different auxlib implementation then there might be issues.
>
> The fix is to simply move the luaL_buffinit call below the lua_getfield call.

Thanks for the correction.

-- Roberto

Reply | Threaded
Open this post in threaded view
|

Re: illegal luaL_buffer usage in loadlib.c

Roberto Ierusalimschy
> > In findloader (https://www.lua.org/source/5.3/loadlib.c.html#findloader)
> > luaL_buffinit is called before pushing the searchers table onto the
> > stack.
> > This breaks the "balanced stack" that luaL_buffinit is documented to expect.
> > Also, later code then assumes that the searchers table is at index 3.
> >
> > Now, this doesn't actually manifest in a bug in PUC Rio lua 5.3, as
> > the implementation of luaL_buffinit doesn't touch the stack.
> > However if someone were to extract parts of loadlib.c to use with a
> > different auxlib implementation then there might be issues.
> >
> > The fix is to simply move the luaL_buffinit call below the lua_getfield call.
>
> Thanks for the correction.

BTW, 'searchpath' has the same issue. (The call to 'luaL_gsub' leaves
a string on the stack.)

-- Roberto

Reply | Threaded
Open this post in threaded view
|

Re: illegal luaL_buffer usage in loadlib.c

Russell Haley
On Wed, Dec 13, 2017 at 4:48 AM, Roberto Ierusalimschy
<[hidden email]> wrote:

>> > In findloader (https://www.lua.org/source/5.3/loadlib.c.html#findloader)
>> > luaL_buffinit is called before pushing the searchers table onto the
>> > stack.
>> > This breaks the "balanced stack" that luaL_buffinit is documented to expect.
>> > Also, later code then assumes that the searchers table is at index 3.
>> >
>> > Now, this doesn't actually manifest in a bug in PUC Rio lua 5.3, as
>> > the implementation of luaL_buffinit doesn't touch the stack.
>> > However if someone were to extract parts of loadlib.c to use with a
>> > different auxlib implementation then there might be issues.
>> >
>> > The fix is to simply move the luaL_buffinit call below the lua_getfield call.
>>
>> Thanks for the correction.
>
> BTW, 'searchpath' has the same issue. (The call to 'luaL_gsub' leaves
> a string on the stack.)
>
> -- Roberto
>

For my edification, if it *could* occur would this be considered a memory leak?

Russ

Reply | Threaded
Open this post in threaded view
|

Re: illegal luaL_buffer usage in loadlib.c

Daurnimator
On 14 December 2017 at 03:57, Russell Haley <[hidden email]> wrote:
> For my edification, if it *could* occur would this be considered a memory leak?

No. It would be an incorrect use of stack.
If a different luaL_buffinit implementation decided to push something
onto the stack unconditionally then e.g. the lua_rawgeti in findloader
would operate on the wrong index (which is likely to not be a table,
and hence likely to crash!)