Repository navigation
Apply max_call_levels to native functions calling back into the engine - #1208
Conversation
|
In that case, should Before, I was under the (wrong) assumption that, since we control all Rust functions, we definitely can avoid any dangerous calls ourselves. Now you've pointed out the fact that certain built-in Rhai functions, such as |
|
Yes, I think it should. You don't need script functions to hit this, just a nested array and ==, so a no_function build overflows the same way. I pushed a commit that drops the no_function gate from set_max_call_levels and max_call_levels (plus the call_stack_depth limit and MAX_CALL_STACK_DEPTH), so the check in _call_fn_raw now runs in that build too, and the regression test isn't gated on no_function anymore. The full test suite passes with and without no_function. Worth keeping in mind that the debug default for max_call_levels is 8, so in a debug build, comparing arrays nested more than 8 deep now needs a higher limit. I left the defaults alone. |
Comparing a deeply nested array overflows the Rust stack and aborts the host, even with the sandbox limits set:
NativeCallContext::_call_fn_rawbumpsglobal.levelon every call from a native function back into the engine, but onlycall_script_fnever compares the level withmax_call_levels. Array==calls==on each element through the context, so it recurses once per nesting level with nothing to stop it. The same goes for map==,contains/index_of, andformat_array/format_mapbehindprint/to_string/debug. Each level costs about 1 KiB of stack in release, so a 9000-deep array (fine undermax_array_size(10_000)) is enough to blow through an 8 MiB main thread, and on a 2 MiB spawned thread it takes about 2000. A stack overflow aborts the process, so no limit orcatch_unwindcan stop it.The fix does the
max_call_levelscheck in_call_fn_rawand returnsErrorStackOverflow, the same check script calls get. Printing still works becauseprint_with_funcalready falls back toDisplaywhen the nested call fails.max_call_levelsandset_max_call_levelsare no longer gated onno_function, since the same recursion happens there too, so the check applies in that build as well. A regression test is intests/stack.rs. Without the fix it aborts the test binary.