Hello, Sergey!
thanks for the patch! LGTM
Sergey
From: Mike Pall <mike> Thanks to Sergey Kaplun. (cherry picked from commit 9d777346bc4e3b033dd78393980d41ee7bc34867) When the number represented the level value is given to the `getfenv()`/`setfenv()`, it is cast to the `int`. Assume the given value is `2^31`, i.e. the resulting value after the cast is `INT_MIN`. After this, it will be decremented in `lj_debug_level()` and underflowed to the `INT_MAX`. That produces the UBSan warning about signed integer overflow. This patch raises the error early in the aforementioned functions, since a negative level value is meaningless. Sergey Kaplun: * added the description and the test for the problem Part of tarantool/tarantool#11055 --- Branch: https://github.com/tarantool/luajit/tree/skaplun/lj-1329-getfenv-setfenv-negative Related issues: * https://github.com/tarantool/tarantool/issues/11055 * https://github.com/LuaJIT/LuaJIT/issues/1329 src/lib_base.c | 4 +++ .../lj-1329-getfenv-setfenv-negative.test.lua | 27 +++++++++++++++++++ 2 files changed, 31 insertions(+) create mode 100644 test/tarantool-tests/lj-1329-getfenv-setfenv-negative.test.lua diff --git a/src/lib_base.c b/src/lib_base.c index ad151975..23728d6c 100644 --- a/src/lib_base.c +++ b/src/lib_base.c @@ -144,6 +144,8 @@ LJLIB_CF(getfenv) LJLIB_REC(.) cTValue *o = L->base; if (!(o < L->top && tvisfunc(o))) { int level = lj_lib_optint(L, 1, 1); + if (level < 0) + lj_err_arg(L, 1, LJ_ERR_INVLVL); o = lj_debug_frame(L, level, &level); if (o == NULL) lj_err_arg(L, 1, LJ_ERR_INVLVL); @@ -166,6 +168,8 @@ LJLIB_CF(setfenv) setgcref(L->env, obj2gco(t)); return 0; } + if (level < 0) + lj_err_arg(L, 1, LJ_ERR_INVLVL); o = lj_debug_frame(L, level, &level); if (o == NULL) lj_err_arg(L, 1, LJ_ERR_INVLVL); diff --git a/test/tarantool-tests/lj-1329-getfenv-setfenv-negative.test.lua b/test/tarantool-tests/lj-1329-getfenv-setfenv-negative.test.lua new file mode 100644 index 00000000..bc10c16f --- /dev/null +++ b/test/tarantool-tests/lj-1329-getfenv-setfenv-negative.test.lua @@ -0,0 +1,27 @@ +local tap = require('tap') + +-- The test file to demonstrate UBSan warning for `setfenv()` and +-- `getfenv()` with a huge `level` value. +-- See also: https://github.com/LuaJIT/LuaJIT/issues/1329. +local test = tap.test('lj-1329-getfenv-setfenv-negative') + +test:plan(4) + +-- This number will be equal to `INT_MIN` when casted to `int`. +-- After this, it will be decremented in `lj_debug_level()` and +-- underflowed to the `INT_MAX`. That produces the UBSan warning +-- about signed integer overflow. +local LEVEL = 2 ^ 31 +local ERRMSG = 'invalid level' + +-- Tests check the UBSan runtime error. Add assertions just to be +-- sure that we don't change the behaviour. +local status, errmsg = pcall(getfenv, LEVEL) +test:ok(not status, 'getfenv: correct status') +test:like(errmsg, ERRMSG, 'getfenv: correct error message') + +status, errmsg = pcall(setfenv, LEVEL, {}) +test:ok(not status, 'setfenv: correct status') +test:like(errmsg, ERRMSG, 'setfenv: correct error message') + +test:done(true)