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

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

[Sergey Kaplun via Tarantool-patches]

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)
-- 
2.45.2

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

[Sergey Bronnikov via Tarantool-patches]

[-- 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 --]

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

[Maxim Kokryashkin via Tarantool-patches]

Hi, Sergey!
Thanks for the patch!
LGTM

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

[Sergey Kaplun via Tarantool-patches]

I've applied the patch into all long-term branches in tarantool/luajit
and bumped a new version in master [1], release/3.2 [2] and
release/2.11 [3].

[1]: https://github.com/tarantool/tarantool/pull/10712
[2]: https://github.com/tarantool/tarantool/pull/10713
[3]: https://github.com/tarantool/tarantool/pull/10714

-- 
Best regards,
Sergey Kaplun