Re: [Tarantool-patches] [PATCH luajit] FFI: Fix __tostring metamethod access to enum cdata value.

[Sergey Bronnikov via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

[-- Attachment #1: Type: text/plain, Size: 2527 bytes --]

Hi, Sergey

thanks for the patch! LGTM

Sergey

On 19.08.2024 14:41, Sergey Kaplun wrote:
> From: Mike Pall <mike>
>
> Thanks to Sergey Kaplun.
>
> (cherry picked from commit f2a1cd43281361035149b6eedbd267b5e71d64d0)
>
> On a 64-bit host, `*(uint32_t **)p` (in the
> `lj_cf_ffi_meta___tostring()`) is the read of 8 bytes, while the size of
> the cdata tail for the enum is only 4. This leads to heap-buffer-overflow
> during the call of `tostring()` on the corresponding cdata.
>
> This patch fixes the pointer cast to `(uint32_t *)p`, which is correct.
>
> Sergey Kaplun:
> * added the description and the test for the problem
>
> Part of tarantool/tarantool#10199
> ---
>
> Branch:https://github.com/tarantool/luajit/tree/skaplun/lj-1232-fix-enum-tostring
> Related issues:
> *https://github.com/LuaJIT/LuaJIT/issues/1232
> *https://github.com/tarantool/tarantool/issues/10199
>
>   src/lib_ffi.c                                 |  2 +-
>   .../lj-1232-fix-enum-tostring.test.lua        | 21 +++++++++++++++++++
>   2 files changed, 22 insertions(+), 1 deletion(-)
>   create mode 100644 test/tarantool-tests/lj-1232-fix-enum-tostring.test.lua
>
> diff --git a/src/lib_ffi.c b/src/lib_ffi.c
> index 3c8dd77f..7988dab8 100644
> --- a/src/lib_ffi.c
> +++ b/src/lib_ffi.c
> @@ -305,7 +305,7 @@ LJLIB_CF(ffi_meta___tostring)
>         p = *(void **)p;
>       } else if (ctype_isenum(ct->info)) {
>         msg = "cdata<%s>: %d";
> -      p = (void *)(uintptr_t)*(uint32_t **)p;
> +      p = (void *)(uintptr_t)*(uint32_t *)p;
>       } else {
>         if (ctype_isptr(ct->info)) {
>   	p = cdata_getptr(p, ct->size);
> diff --git a/test/tarantool-tests/lj-1232-fix-enum-tostring.test.lua b/test/tarantool-tests/lj-1232-fix-enum-tostring.test.lua
> new file mode 100644
> index 00000000..073bfcb6
> --- /dev/null
> +++ b/test/tarantool-tests/lj-1232-fix-enum-tostring.test.lua
> @@ -0,0 +1,21 @@
> +local tap = require('tap')
> +
> +-- Test file to demonstrate heap-buffer-overflow in the
> +-- `tostring()` call for the enum cdata.
> +-- See also:https://github.com/LuaJIT/LuaJIT/issues/1232.
> +
> +local test = tap.test('lj-1232-fix-enum-tostring')
> +
> +local ffi = require('ffi')
> +local ENUM_VAL = 1
> +local EXPECTED = 'cdata<enum %d+>: ' .. ENUM_VAL
> +
> +test:plan(1)
> +
> +local cdata_enum = ffi.new(('enum {foo = %d}'):format(ENUM_VAL), ENUM_VAL)
> +
> +-- XXX: The test shows heap-buffer-overflow only under ASAN.
> +
> +test:like(tostring(cdata_enum), EXPECTED, 'correct tostring result')
> +
> +test:done(true)

[-- Attachment #2: Type: text/html, Size: 3298 bytes --]