Skip to content

Do not update _top before resizing _stack#321

Open
unknown-1-0 wants to merge 1 commit into
albertodemichelis:masterfrom
unknown-1-0:fix-heap-bof-after-stack-overflow-in-a-metamethod
Open

Do not update _top before resizing _stack#321
unknown-1-0 wants to merge 1 commit into
albertodemichelis:masterfrom
unknown-1-0:fix-heap-bof-after-stack-overflow-in-a-metamethod

Conversation

@unknown-1-0

Copy link
Copy Markdown

When SQVM::EnterFrame is called, it updates the value of _top BEFORE checking if that new value is within _stack. When this function encounters a stack overflow while in a metamethod, it returns without reverting the previous value of _top. This leads to _top still pointing outside the bounds of _stack, which later leads to out-of-bounds access and heap corruption.
The reproducer code is provided below:

import sys

if len(sys.argv) < 3:
    print("Usage: ./gen_locals_stack_overflow_repro.py <number of locals inside _get> <number of locals outside _get>")
    exit(1)

locals_inside_get = int(sys.argv[1])
locals_outside_get = int(sys.argv[2])

print(f"Locals inside _get: {locals_inside_get}")
print(f"Locals outside _get: {locals_outside_get}")


out_path="locals_stack_overflow.nut"
f=open(out_path,"w")

f.write("""local a = {
    calls = 0
    function _get(x) {
        calls += 1;
        print("Call #" + calls + ", x = " + x + "\\n");
        nonexistent_var_2; // triggers _get(x) with x = nonexistent_var_2

""")

for i in range(locals_inside_get):
    f.write(" "*4*2 + f"local _{i:02x} = {hex(i)}\n")

f.write(" "*4 + "}" + """
}
""")

for i in range(locals_outside_get):
    f.write(f"local _{i:02x} = {hex(i)}\n")

f.write("""

local b = setdelegate(a)
local b = nonexistent_var // triggers _get(x) with x = nonexistent_var
""")

f.close()

print(f"Repro saved to {out_path}")
$ python ./gen_locals_stack_overflow_repro.py 254 0
Locals inside _get: 254
Locals outside _get: 0
Repro saved to locals_stack_overflow.nut
$ mkdir build-asan
$ cd build-asan
$ CFLAGS='-fsanitize=address,undefined' CXXFLAGS='-fsanitize=address,undefined' cmake .. -DCMAKE_BUILD_TYPE=Debug
<cmake output omitted for brevity>
$ make
<make output omitted for brevity>
$ ./bin/sq ../locals_stack_overflow.nut # assuming the python script was run in the root of the repo
Call #1, x = nonexistent_var
Call #2, x = nonexistent_var_2
Call #3, x = nonexistent_var_2
=================================================================
==<sq PID here>==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x7de54d5e4270 at pc 0x7f554f42a664 bp 0x7fff835f4620 sp 0x7fff835f4618
READ of size 4 at 0x7de54d5e4270 thread T0                                                                                                                                      
    #0 0x7f554f42a663 in SQObjectPtr::Null() <...>/squirrel/squirrel/sqobject.h:289
    #1 0x7f554f4c0f15 in SQVM::Pop(long long) <...>/squirrel/squirrel/sqvm.cpp:1761
    #2 0x7f554f4e5100 in SQVM::FallBackGet(SQObjectPtr const&, SQObjectPtr const&, SQObjectPtr&) <...>/squirrel/squirrel/sqvm.cpp:1366
    #3 0x7f554f4e6049 in SQVM::Get(SQObjectPtr const&, SQObjectPtr const&, SQObjectPtr&, unsigned long long, long long) <...>/squirrel/squirrel/sqvm.cpp:1299
    #4 0x7f554f4cbc87 in SQVM::Execute(SQObjectPtr&, long long, long long, SQObjectPtr&, unsigned long long, SQVM::ExecutionType) <...>/squirrel/squirrel/sqvm.cpp:853
    #5 0x7f554f4e06e6 in SQVM::Call(SQObjectPtr&, long long, long long, SQObjectPtr&, unsigned long long) <...>/squirrel/squirrel/sqvm.cpp:1610
    #6 0x7f554f4e50d8 in SQVM::FallBackGet(SQObjectPtr const&, SQObjectPtr const&, SQObjectPtr&) <...>/squirrel/squirrel/sqvm.cpp:1361
    #7 0x7f554f4e6049 in SQVM::Get(SQObjectPtr const&, SQObjectPtr const&, SQObjectPtr&, unsigned long long, long long) <...>/squirrel/squirrel/sqvm.cpp:1299
    #8 0x7f554f4cbc87 in SQVM::Execute(SQObjectPtr&, long long, long long, SQObjectPtr&, unsigned long long, SQVM::ExecutionType) <...>/squirrel/squirrel/sqvm.cpp:853
    #9 0x7f554f4e06e6 in SQVM::Call(SQObjectPtr&, long long, long long, SQObjectPtr&, unsigned long long) <...>/squirrel/squirrel/sqvm.cpp:1610
    #10 0x7f554f4e50d8 in SQVM::FallBackGet(SQObjectPtr const&, SQObjectPtr const&, SQObjectPtr&) <...>/squirrel/squirrel/sqvm.cpp:1361
    #11 0x7f554f4e6049 in SQVM::Get(SQObjectPtr const&, SQObjectPtr const&, SQObjectPtr&, unsigned long long, long long) <...>/squirrel/squirrel/sqvm.cpp:1299
    #12 0x7f554f4cbc87 in SQVM::Execute(SQObjectPtr&, long long, long long, SQObjectPtr&, unsigned long long, SQVM::ExecutionType) <...>/squirrel/squirrel/sqvm.cpp:853
    #13 0x7f554f4e06e6 in SQVM::Call(SQObjectPtr&, long long, long long, SQObjectPtr&, unsigned long long) <...>/squirrel/squirrel/sqvm.cpp:1610
    #14 0x7f554f4e50d8 in SQVM::FallBackGet(SQObjectPtr const&, SQObjectPtr const&, SQObjectPtr&) <...>/squirrel/squirrel/sqvm.cpp:1361
    #15 0x7f554f4e6049 in SQVM::Get(SQObjectPtr const&, SQObjectPtr const&, SQObjectPtr&, unsigned long long, long long) <...>/squirrel/squirrel/sqvm.cpp:1299
    #16 0x7f554f4cbc87 in SQVM::Execute(SQObjectPtr&, long long, long long, SQObjectPtr&, unsigned long long, SQVM::ExecutionType) <...>/squirrel/squirrel/sqvm.cpp:853
    #17 0x7f554f4e06e6 in SQVM::Call(SQObjectPtr&, long long, long long, SQObjectPtr&, unsigned long long) <...>/squirrel/squirrel/sqvm.cpp:1610
    #18 0x7f554f41bc05 in sq_call <...>/squirrel/squirrel/sqapi.cpp:1178
    #19 0x55d51ea55245 in getargs <...>/squirrel/sq/sq.c:193
    #20 0x55d51ea56137 in main <...>/squirrel/sq/sq.c:330
    #21 0x7f554e629f67 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
    #22 0x7f554e62a024 in __libc_start_main_impl ../csu/libc-start.c:360
    #23 0x55d51ea54420 in _start (<...>/squirrel/build-asan-gcc/bin/sq+0x4420) (BuildId: 3d33fe8dc07ff2144c2c0957793b58cb63a6d328)

0x7de54d5e4270 is located 112 bytes after 16384-byte region [0x7de54d5e0200,0x7de54d5e4200)
allocated by thread T0 here:                                                                                                                                                    
    #0 0x7f554f918fcb in realloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:81
    #1 0x7f554f4950a1 in sq_vm_realloc(void*, unsigned long long, unsigned long long) <...>/squirrel/squirrel/sqmem.cpp:8
    #2 0x7f554f42ee73 in sqvector<SQObjectPtr>::_realloc(unsigned long long) <...>/squirrel/squirrel/squtils.h:110
    #3 0x7f554f42f00f in sqvector<SQObjectPtr>::resize(unsigned long long, SQObjectPtr const&) <...>/squirrel/squirrel/squtils.h:58
    #4 0x7f554f4c34c1 in SQVM::Init(SQVM*, long long) <...>/squirrel/squirrel/sqvm.cpp:357
    #5 0x7f554f40eaba in sq_open <...>/squirrel/squirrel/sqapi.cpp:51
    #6 0x55d51ea560d3 in main <...>/squirrel/sq/sq.c:314
    #7 0x7f554e629f67 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58

SUMMARY: AddressSanitizer: heap-buffer-overflow <...>/squirrel/squirrel/sqobject.h:289 in SQObjectPtr::Null()
Shadow bytes around the buggy address:
  0x7de54d5e3f80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x7de54d5e4000: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x7de54d5e4080: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x7de54d5e4100: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x7de54d5e4180: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x7de54d5e4200: fa fa fa fa fa fa fa fa fa fa fa fa fa fa[fa]fa
  0x7de54d5e4280: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x7de54d5e4300: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x7de54d5e4380: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x7de54d5e4400: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x7de54d5e4480: 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
==<sq PID here>==ABORTING

This pull request fixes this bug by updating the value of _top AFTER the new top value is checked and _stack is resized (if needed).
Below is the output of the same reproducer run on squirrel with the patch applied and ASAN enabled:

$ cd build-asan
$ make
<make output omitted for brevity>
$ ./bin/sq ../locals_stack_overflow.nut
Call #1, x = nonexistent_var
Call #2, x = nonexistent_var_2
Call #3, x = nonexistent_var_2

AN ERROR HAS OCCURRED [stack overflow, cannot resize stack while in a metamethod]

CALLSTACK
*FUNCTION [main()] ../locals_stack_overflow.nut line [267]

LOCALS
[b] TABLE
[a] TABLE
[vargv] ARRAY
[this] TABLE
$ 

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant