Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Incorrect PC value in a function predict_next #1054

Closed
ligurio opened this issue Aug 23, 2023 · 2 comments
Closed

Incorrect PC value in a function predict_next #1054

ligurio opened this issue Aug 23, 2023 · 2 comments
Labels
2.0 2.1 bug

Comments

@ligurio
Copy link

ligurio commented Aug 23, 2023 •
edited

On running following Lua code:

a, b, c = 1, 2, 3
local d
for _ in nil do end

by LuaJIT, instrumented with Address Sanitizer, heap-buffer-overflow is observed:

=================================================================
==1066023==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x506000000c60 at pc 0x560d37d16ea3 bp 0x7ffeb8ba7dd0 sp 0x7ffeb8ba7dc8
READ of size 4 at 0x506000000c60 thread T0
    #0 0x560d37d16ea2  (/home/sergeyb/sources/lua-c-api-tests/build/luajit-v2.1/source/src/luajit+0x1f7ea2) (BuildId: 4287065d61612312cc078b2572b5a4b43a0fabd2)
    #1 0x560d37d0bd91  (/home/sergeyb/sources/lua-c-api-tests/build/luajit-v2.1/source/src/luajit+0x1ecd91) (BuildId: 4287065d61612312cc078b2572b5a4b43a0fabd2)
    #2 0x560d37d3250d  (/home/sergeyb/sources/lua-c-api-tests/build/luajit-v2.1/source/src/luajit+0x21350d) (BuildId: 4287065d61612312cc078b2572b5a4b43a0fabd2)
    #3 0x560d37d62f33  (/home/sergeyb/sources/lua-c-api-tests/build/luajit-v2.1/source/src/luajit+0x243f33) (BuildId: 4287065d61612312cc078b2572b5a4b43a0fabd2)
    #4 0x560d37d3307d in luaL_loadfilex (/home/sergeyb/sources/lua-c-api-tests/build/luajit-v2.1/source/src/luajit+0x21407d) (BuildId: 4287065d61612312cc078b2572b5a4b43a0fabd2)
    #5 0x560d37cc87a3  (/home/sergeyb/sources/lua-c-api-tests/build/luajit-v2.1/source/src/luajit+0x1a97a3) (BuildId: 4287065d61612312cc078b2572b5a4b43a0fabd2)
    #6 0x560d37d62b3d  (/home/sergeyb/sources/lua-c-api-tests/build/luajit-v2.1/source/src/luajit+0x243b3d) (BuildId: 4287065d61612312cc078b2572b5a4b43a0fabd2)
    #7 0x560d37d01bdf in lua_cpcall (/home/sergeyb/sources/lua-c-api-tests/build/luajit-v2.1/source/src/luajit+0x1e2bdf) (BuildId: 4287065d61612312cc078b2572b5a4b43a0fabd2)
    #8 0x560d37cc799c in main (/home/sergeyb/sources/lua-c-api-tests/build/luajit-v2.1/source/src/luajit+0x1a899c) (BuildId: 4287065d61612312cc078b2572b5a4b43a0fabd2)
    #9 0x7f6e1f229d8f in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16
    #10 0x7f6e1f229e3f in __libc_start_main csu/../csu/libc-start.c:392:3
    #11 0x560d37bef604 in _start (/home/sergeyb/sources/lua-c-api-tests/build/luajit-v2.1/source/src/luajit+0xd0604) (BuildId: 4287065d61612312cc078b2572b5a4b43a0fabd2)

0x506000000c60 is located 0 bytes after 64-byte region [0x506000000c20,0x506000000c60)
allocated by thread T0 here:
    #0 0x560d37c8b245 in realloc (/home/sergeyb/sources/lua-c-api-tests/build/luajit-v2.1/source/src/luajit+0x16c245) (BuildId: 4287065d61612312cc078b2572b5a4b43a0fabd2)
    #1 0x560d37d6c7ef  (/home/sergeyb/sources/lua-c-api-tests/build/luajit-v2.1/source/src/luajit+0x24d7ef) (BuildId: 4287065d61612312cc078b2572b5a4b43a0fabd2)
    #2 0x560d37d6cbd9  (/home/sergeyb/sources/lua-c-api-tests/build/luajit-v2.1/source/src/luajit+0x24dbd9) (BuildId: 4287065d61612312cc078b2572b5a4b43a0fabd2)
    #3 0x560d37d0ca08  (/home/sergeyb/sources/lua-c-api-tests/build/luajit-v2.1/source/src/luajit+0x1eda08) (BuildId: 4287065d61612312cc078b2572b5a4b43a0fabd2)
    #4 0x560d37d0bd81  (/home/sergeyb/sources/lua-c-api-tests/build/luajit-v2.1/source/src/luajit+0x1ecd81) (BuildId: 4287065d61612312cc078b2572b5a4b43a0fabd2)
    #5 0x560d37d3250d  (/home/sergeyb/sources/lua-c-api-tests/build/luajit-v2.1/source/src/luajit+0x21350d) (BuildId: 4287065d61612312cc078b2572b5a4b43a0fabd2)
    #6 0x560d37d62f33  (/home/sergeyb/sources/lua-c-api-tests/build/luajit-v2.1/source/src/luajit+0x243f33) (BuildId: 4287065d61612312cc078b2572b5a4b43a0fabd2)

SUMMARY: AddressSanitizer: heap-buffer-overflow (/home/sergeyb/sources/lua-c-api-tests/build/luajit-v2.1/source/src/luajit+0x1f7ea2) (BuildId: 4287065d61612312cc078b2572b5a4b43a0fabd2) 
Shadow bytes around the buggy address:
  0x506000000980: 00 00 00 00 00 00 00 00 fa fa fa fa 00 00 00 00
  0x506000000a00: 00 00 00 00 fa fa fa fa 00 00 00 00 00 00 00 00
  0x506000000a80: fa fa fa fa 00 00 00 00 00 00 00 00 fa fa fa fa
  0x506000000b00: 00 00 00 00 00 00 00 00 fa fa fa fa 00 00 00 00
  0x506000000b80: 00 00 00 00 fa fa fa fa 00 00 00 00 00 00 00 00
=>0x506000000c00: fa fa fa fa 00 00 00 00 00 00 00 00[fa]fa fa fa
  0x506000000c80: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x506000000d00: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x506000000d80: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x506000000e00: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x506000000e80: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07 
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
==1066023==ABORTING

Without ASAN LuaJIT reports a warning:

$ ./build/luajit-v2.1/source/src/luajit crash-6e1698df576265af3cf31c0733effc109c004c77.lua 
./build/luajit-v2.1/source/src/luajit: crash-6e1698df576265af3cf31c0733effc109c004c77.lua:3: attempt to call a nil value
stack traceback:
        crash-6e1698df576265af3cf31c0733effc109c004c77.lua:3: in main chunk
        [C]: at 0x5592ad2f57d0

Affected source code:

LuaJIT/src/lj_parse.c

Line 2516 in 8635cba

BCIns ins = fs->bcbase[pc].ins;

Proposed patch:

diff --git a/src/lj_parse.c b/src/lj_parse.c
index 343fa797..a843dcb3 100644
--- a/src/lj_parse.c
+++ b/src/lj_parse.c
@@ -2511,7 +2511,7 @@ static void parse_for_num(LexState *ls, GCstr *varname, BCLine line)
 */
 static int predict_next(LexState *ls, FuncState *fs, BCPos pc)
 {
-  BCIns ins = fs->bcbase[pc].ins;
+  BCIns ins = fs->bcbase[pc >= fs->bclim ? fs->bclim - 1 : pc].ins;
   GCstr *name;
   cTValue *o;
   switch (bc_op(ins)) {

Thanks to Sergey Kaplun @Buristan for help with investigation.
@Buristan
Copy link

Just few comments to add: The reason is similar with #1033 -- exprpc looks forward and expect extra bytecodes on the stack. But since KPRI is merged to the KNIL -- there is no new bytecode to add, so exprpc == fs->bclim and looks out of stack bounds.

MikePall pushed a commit that referenced this issue Aug 29, 2023
Fix predict_next() in parser (again).
309fb42 
Reported by Sergey Bronnikov. #1054
@MikePall
Copy link
Member

Fixed in a different way. Thanks!

ligurio pushed a commit to tarantool/luajit that referenced this issue Aug 29, 2023
@ligurio
Fix predict_next() in parser (again).
8b2d744 
Reported by Sergey Bronnikov. LuaJIT#1054

(cherry picked from commit 309fb42)

The following Lua snippet triggers an out of boundary access to a stack:

```lua
a, b, c = 1, 2, 3
local d
for _ in nil do end
```

With execution snippet by LuaJIT instrumented by ASAN it leads to
a heap-buffer-overflow.

In a function `predict_next` variable `exprpc` looks forward and expects
extra bytecodes on the stack. However, `KPRI` is merged to the `KNIL`
and there is no new bytecode to add, so `exprpc == fs->bclim` and it
leads to out of boundary access.

Sergey Bronnikov:
* added the description and the test for the problem

Part of tarantool/tarantool#8825
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
2.0 2.1 bug
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@MikePall @ligurio @Buristan