[Tarantool-patches] [PATCH luajit 16/19] Prevent integer overflow while parsing long strings.

[Sergey Bronnikov]

Hi, Sergey!


thanks for the patch!

test duration is about 7 sec, I propose to add a print before string.rep:

print('# test generation requires about 7 sec')

Otherwise it looks like test hang. Feel free to ignore.


LGTM after fixing comments from Max.


Sergey

On 8/9/23 18:36, Sergey Kaplun wrote:
> From: Mike Pall <mike>
>
> (cherry-picked from commit 16e5605eec2e3882d709c6b123a644f6a8023945)
>
> This commit fixes possible integer overflow of the separator's length
> counter during parsing long strings. It may lead to the fact, that
> parser considers a string with unbalanced long brackets to be correct.
> Since this is pointless to parse too long string separators in the hope,
> that the string is correct, just use hardcoded limit (2 ^ 25 is enough).
>
> Be aware that this limit is different for Lua 5.1.
>
> We can't check the string overflow itself without a really large file,
> because the ERR_MEM error will be raised, due to the string buffer
> reallocations during parsing. Keep such huge file in the repo is
> pointless, so just check that we don't parse long string after
> aforementioned separator length.
>
> Sergey Kaplun:
> * added the description and the test for the problem
>
> Part of tarantool/tarantool#8825
> ---
>   src/lj_lex.c                                  |  2 +-
>   .../lj-812-too-long-string-separator.test.lua | 31 +++++++++++++++++++
>   2 files changed, 32 insertions(+), 1 deletion(-)
>   create mode 100644 test/tarantool-tests/lj-812-too-long-string-separator.test.lua
>
> diff --git a/src/lj_lex.c b/src/lj_lex.c
> index 52856912..c66660d7 100644
> --- a/src/lj_lex.c
> +++ b/src/lj_lex.c
> @@ -138,7 +138,7 @@ static int lex_skipeq(LexState *ls)
>     int count = 0;
>     LexChar s = ls->c;
>     lua_assert(s == '[' || s == ']');
> -  while (lex_savenext(ls) == '=')
> +  while (lex_savenext(ls) == '=' && count < 0x20000000)
>       count++;
>     return (ls->c == s) ? count : (-count) - 1;
>   }
> diff --git a/test/tarantool-tests/lj-812-too-long-string-separator.test.lua b/test/tarantool-tests/lj-812-too-long-string-separator.test.lua
> new file mode 100644
> index 00000000..fda69d17
> --- /dev/null
> +++ b/test/tarantool-tests/lj-812-too-long-string-separator.test.lua
> @@ -0,0 +1,31 @@
> +local tap = require('tap')
> +
> +-- Test to check that we avoid parsing of too long separator
> +-- for long strings.
> +-- See also the discussion in the
> +-- https://github.com/LuaJIT/LuaJIT/issues/812.
> +
> +local test = tap.test('lj-812-too-long-string-separator'):skipcond({
> +  ['Test requires GC64 mode enabled'] = not require('ffi').abi('gc64'),
> +})
> +test:plan(2)
> +
> +-- We can't check the string overflow itself without a really
> +-- large file, because the ERR_MEM error will be raised, due to
> +-- the string buffer reallocations during parsing.
> +-- Keep such huge file in the repo is pointless, so just check
> +-- that we don't parse long string after some separator length.
> +-- Be aware that this limit is different for Lua 5.1.
> +
> +-- Use the hardcoded limit. The same as in the <src/lj_lex.c>.
> +local separator = string.rep('=', 0x20000000 + 1)
> +local test_str = ('return [%s[]%s]'):format(separator, separator)
> +
> +local f, err = loadstring(test_str, 'empty_str_f')
> +test:ok(not f, 'correct status when parsing string with too long separator')
> +
> +-- Check error message.
> +test:ok(tostring(err):match('invalid long string delimiter'),
> +        'correct error when parsing string with too long separator')
> +
> +test:done(true)