luaX_token2str() error: buffer overflow 'luai_ctype_'

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

luaX_token2str() error: buffer overflow 'luai_ctype_'

Toomas Soome
hi!

while doing static code analyze with smatch, the following issue is reported:

    /code/illumos-gate/usr/src/tools/proto/root_i386-nd/opt/onbld/bin/i386/smatch: ../../common/fs/zfs/lua/llex.c:76 luaX_token2str() error: buffer overflow 'luai_ctype_' 257 <= 257

meaning, it is possible, we would attempt to read memory with luai_ctype_[257]. This can happen despite the check ‘if (token < FIRST_RESERVED)’ because of call lisprint(token + 1).

The simple idea of fix would be:

--- a/usr/src/uts/common/fs/zfs/lua/llex.c
+++ b/usr/src/uts/common/fs/zfs/lua/llex.c
@@ -71,7 +71,7 @@ void luaX_init (lua_State *L) {

 

 

 const char *luaX_token2str (LexState *ls, int token) {
-  if (token < FIRST_RESERVED) {  /* single-byte symbols? */
+  if (token < FIRST_RESERVED - 1) {  /* single-byte symbols? */
     lua_assert(token == cast(unsigned char, token));
     return (lisprint(token)) ? luaO_pushfstring(ls->L, LUA_QL("%c"), token) :
                               luaO_pushfstring(ls->L, "char(%d)", token);

but I’m not too familiar about lua internals, so it might not be quite correct…

rgds,
toomas
Reply | Threaded
Open this post in threaded view
|

Re: luaX_token2str() error: buffer overflow 'luai_ctype_'

Luiz Henrique de Figueiredo
> it is possible, we would attempt to read memory with luai_ctype_[257]

llex returns single-byte tokens (at most 255) or composite tokens
starting at FIRST_RESERVED, which is 257. Therefore, a token is never
256 and luai_ctype_[257] is never read.
Reply | Threaded
Open this post in threaded view
|

Re: luaX_token2str() error: buffer overflow 'luai_ctype_'

Toomas Soome


On 9. Sep 2020, at 14:42, Luiz Henrique de Figueiredo <[hidden email]> wrote:

it is possible, we would attempt to read memory with luai_ctype_[257]

llex returns single-byte tokens (at most 255) or composite tokens
starting at FIRST_RESERVED, which is 257. Therefore, a token is never
256 and luai_ctype_[257] is never read.

Yes, You know that, compiler does not. Compiler only does know that we enter luaX_token2str(LexState *ls, int token) and int is 32-bit value, meaning that it is possible to construct code path with token == 256, and in such case, we read outside array bounds.

So, as the assumption is that 256 is impossible value, we can set up assert or condition to test that value.

rgds,
toomas
Reply | Threaded
Open this post in threaded view
|

Re: luaX_token2str() error: buffer overflow 'luai_ctype_'

Luiz Henrique de Figueiredo
> So, as the assumption is that 256 is impossible value, we can set up assert or condition to test that value.

The code already has this:
     lua_assert(token == cast(unsigned char, token));
Reply | Threaded
Open this post in threaded view
|

Re: luaX_token2str() error: buffer overflow 'luai_ctype_'

Toomas Soome


On 9. Sep 2020, at 15:22, Luiz Henrique de Figueiredo <[hidden email]> wrote:

So, as the assumption is that 256 is impossible value, we can set up assert or condition to test that value.

The code already has this:
    lua_assert(token == cast(unsigned char, token));

yep. it does. Also it has 

#define lua_assert(c)         ((void)0)

Which does explain the issue about smatch not seeing the check, and of course, it also means, unless the data segment layout is not favoring us, no-one will ever know if the bug did hit or not:)

thanks,
toomas