[Tarantool-patches] [PATCH luajit] Fix zero stripping in %g number formatting.
Maxim Kokryashkin
m.kokryashkin at tarantool.org
Wed Jan 31 12:42:13 MSK 2024
Hi, Sergey!
Thanks for the patch!
LGTM, except for two nits regarding the commit message,
and two nits regarding the test case comment.
On Tue, Jan 30, 2024 at 06:04:37PM +0300, Sergey Kaplun wrote:
> From: Mike Pall <mike>
>
> Reported by pwnhacker0x18.
>
> (cherry picked from commit 343ce0edaf3906a62022936175b2f5410024cbfc)
>
> In the situation when the precision (`prec`) and amount of digits
> (`hilen`) for the decimal representation are the same and `ndhi` == 0,
> the `ndlo` part will become 64 (the size of the `nd` stack buffer), and
Typo: s/will become/becomes/
> the overflow occurs.
>
> This patch adds the corresponding mask (0x3f == 63) for the `ndlo`
> incrementation result.
Please mention that all of this happens in the `lj_strfmt_wfnum`
function in the commit message.
>
> Sergey Kaplun:
> * added the description and the test for the problem
>
> Part of tarantool/tarantool#9595
> ---
>
> Branch: https://github.com/tarantool/luajit/tree/skaplun/lj-1149-g-number-formating
> Tarantool PR: https://github.com/tarantool/tarantool/pull/9633
>
> The test <app-tap/gh-2717-no-quit-sigint.test.lua> fails on M1 with the
> timeout (see the example [1]). This fail is patch-unrelated, since I've
> obscured this failure even for the branch without sources changes (tests
> only).
>
> Related Issues:
> * https://github.com/LuaJIT/LuaJIT/issues/1149
> * https://github.com/tarantool/tarantool/issues/9595
>
> [1]: https://github.com/tarantool/luajit/actions/runs/7712549489/job/21020513973#step:8:5522
>
> Duration of failed tests (seconds):
> * 60.54 app-tap/gh-2717-no-quit-sigint.test.lua
>
> src/lj_strfmt_num.c | 3 ++-
> .../lj-1149-g-number-formating-bufov.test.lua | 20 +++++++++++++++++++
> 2 files changed, 22 insertions(+), 1 deletion(-)
> create mode 100644 test/tarantool-tests/lj-1149-g-number-formating-bufov.test.lua
>
> diff --git a/src/lj_strfmt_num.c b/src/lj_strfmt_num.c
> index c26204b7..c8d9febf 100644
> --- a/src/lj_strfmt_num.c
> +++ b/src/lj_strfmt_num.c
> @@ -454,7 +454,8 @@ static char *lj_strfmt_wfnum(SBuf *sb, SFormat sf, lua_Number n, char *p)
> prec--;
> if (!i) {
> if (ndlo == ndhi) { prec = 0; break; }
> - lj_strfmt_wuint9(tail, nd[++ndlo]);
> + ndlo = (ndlo + 1) & 0x3f;
> + lj_strfmt_wuint9(tail, nd[ndlo]);
> i = 9;
> }
> }
> diff --git a/test/tarantool-tests/lj-1149-g-number-formating-bufov.test.lua b/test/tarantool-tests/lj-1149-g-number-formating-bufov.test.lua
> new file mode 100644
> index 00000000..040fd5de
> --- /dev/null
> +++ b/test/tarantool-tests/lj-1149-g-number-formating-bufov.test.lua
> @@ -0,0 +1,20 @@
> +local tap = require('tap')
> +
> +-- Test file to demonstrate stack-buffer-overflow in the
> +-- `lj_strfmt_wfnum()` call.
> +-- See also: https://github.com/LuaJIT/LuaJIT/issues/1149.
> +
> +local test = tap.test('lj-1149-g-number-formating-bufov')
> +test:plan(1)
> +
> +-- XXX: The test shows stack-buffer-overflow only under ASAN.
> +-- The number value for the test is with the same precision
Typo: s/is with/has/
> +-- (`prec` = 5) and amount of digits (`hilen` = 5) for the decimal
> +-- representation. Hence, with `ndhi` == 0, the `ndlo` part will
> +-- become 64 (the size of the `nd` stack buffer), and the overflow
Typo: s/will become/becomes/
> +-- occurs.
> +-- See details in the <src/lj_strfmt_num.c>:`lj_strfmt_wfnum()`.
> +test:is(string.format('%7g', 0x1.144399609d407p+401), '5.5733e+120',
> + 'correct format %7g result')
> +
> +test:done(true)
> --
> 2.43.0
>
More information about the Tarantool-patches
mailing list