Moddable-OpenSource / moddable

Tools for developers to create truly open IoT products using standard JavaScript on low cost microcontrollers.
http://www.moddable.com
1.34k stars 236 forks source link

Multiple stack overflows due to no internal recursive limit on prototype/builtin functions. #894

Closed 0x41c closed 2 years ago

0x41c commented 2 years ago

Build environment: macOS Target: SDK

Description The lack of an internal recursive limit allows a stack overflow to occur when a prototype/built-in function has the parent function in the stack passed in as an argument to it.

Steps to Reproduce Here's one of them (I have like 10 more if needed)

const array = [0];
function recurse(self) {
    -self; // (2...) In an attempt to get the primative this calls "getOwnPropertyDescriptor" which calls "recurse" yet again.
    const proxy = new Proxy(array, {
        "getOwnPropertyDescriptor": recurse,
    });
    for (const descriptor in proxy) {} // (1) Trigger call to "getOwnPropertyDescriptor" which calls "recurse"
}
recurse(0);

I ran this with XST but you can use XS and just have it never return if that tickles your fancy :)

Expected behavior After a certain amount of recursive calls, there should be termination of the program with a message explaining where the stack-overflow occurred. A traceback might help to debug as well.

Other information Here's an ASAN log with some calls to fxRunID and unimportant details omitted:

xst(3178,0x1124ff600) malloc: nano zone abandoned due to inability to preallocate reserved vm space.
[COV] no shared memory bitmap available, skipping
[COV] edge counters initialized. Shared memory: (null) with 22810 edges
AddressSanitizer:DEADLYSIGNAL
=================================================================
==3178==ERROR: AddressSanitizer: stack-overflow on address 0x7ff7b11c0ab8 (pc 0x00010e828c45 bp 0x7ff7b11cf2a0 sp 0x7ff7b11c0ac0 T0)
    #0 0x10e828c45 in fxRunID xsRun.c:380
    #1 0x10e5982a0 in fx_Array_prototype_toString xsArray.c:2500
    #2 0x10e82ce6c in fxRunID xsRun.c:842
    #3 0x10e93339f in fxOrdinaryToPrimitive xsType.c:889
    #4 0x10e82ce6c in fxRunID xsRun.c:842
    #5 0x10e93186f in fxToPrimitive xsType.c:269
    #6 0x10e89db9a in fxToNumericNumber xsRun.c:4666
    #7 0x10e89d9d4 in fxToNumericNumberUnary xsRun.c:4675
    #8 0x10e874dce in fxRunID xsRun.c:3093
    #9 0x10e7f276d in fxProxyGetOwnProperty xsProxy.c:357
    #10 0x10e6bc473 in fx_Enumerator_next xsGlobal.c:358
    #11 0x10e82ce6c in fxRunID xsRun.c:842
    #12 0x10e7f276d in fxProxyGetOwnProperty xsProxy.c:357
    #13 0x10e6bc473 in fx_Enumerator_next xsGlobal.c:358
    #14 0x10e82ce6c in fxRunID xsRun.c:842
    #15 0x10e7f276d in fxProxyGetOwnProperty xsProxy.c:357
    #16 0x10e6bc473 in fx_Enumerator_next xsGlobal.c:358
    ...

The paths above should have a check to see what the stack depth is and bail when it's too deep of an internal recursive stack.

This isn't a serious issue but it doesn't hurt to have it patched up to keep it consistent with some other relevant engines.

phoddie commented 2 years ago

Thank you for the report.

I agree that XS should not crash in this scenario. In my tests, it does not. Here's a run of the sample script above on macOS, with ASAN enabled:

$ xst -s ~/test.js 
xst(38733,0x11e4e5600) malloc: nano zone abandoned due to inability to preallocate reserved vm space.
Error: stack overflow

There is a check for possible native stack overflow near the start of fxRunID. While not directly relevant here, there are also checks throughout XS when pushing to the JavaScript stack (for example, see the use of fxOverflow in xsAll.h).

Perhaps you have xst configured a way that bypasses or invalidates the native stack checks?

0x41c commented 2 years ago

I'm sure I botched this example while testing. I found myself executing other examples by accident as well, so that could have happened here. I'll get on my computer today and either fix that one, or post a separate POC. Thanks for notifying me that it doesn't work!

phoddie commented 2 years ago

No problem. Thanks for taking another look.

phoddie commented 2 years ago

FWIW – we saw a similar issue where a native stack overflow did not appear to be caught. It looks like the actual problem is ASAN getting in the way. Bumping mxASANStackMargin to 65536 appears to fix the problem, allowing for a clean exit on native stack overflows. That change will be included in the next Moddable SDK update.

0x41c commented 2 years ago

Really that's interesting, lemme me give you something to test, I want to see if this works.

const array = [0];
function recurse() {
    array.every(recurse);
}
recurse();
phoddie commented 2 years ago

That works – clean exit with Error: stack overflow. I modified it as follows to see what it was doing:

const array = [0];
let count = 0;
function recurse() {
print("recurse " + ++count)
    array.every(recurse);
}
recurse();

Output ends here:

...
recurse 134
recurse 135
recurse 136
Error: stack overflow
0x41c commented 2 years ago

Nice! Glad to see this resolved. This issue can be closed now.