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

Allow DUK_USE_EXEC_TIMEOUT_CHECK to be called from within the main loop of a long lived regular expression #2199

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

braindigitalis
Copy link

I am using duktape in an environment where it may be asked to run untrusted code as part of a platform as a service, and the function duk__match_regexp() can potentially run for a very long time if an abusive or broken regular expression is passed to it. This is somewhat mitigated by the value of DUK_RE_EXECUTE_STEPS_LIMIT, lowering this can solve the problem but it seems this cannot be lowered from duk_config.h, and i would have to edit my duktape.c to fix it.

Based upon this i decided on this fix instead - i am already using DUK_USE_EXEC_TIMEOUT_CHECK with an interrupt function to prevent cpu consumption and various forms of resource abuse, but it seems the timeout function is not called from within the duk__match_regexp() function.

This PR adds support for calling the interrupt function from within the loop, this doesnt change any specification i'm aware of, as the docs say just that the interrupt function is 'called periodically' while running the script.

Please consider either merging this PR in some form, or allowing user configuration of DUK_RE_EXECUTE_STEPS_LIMIT and DUK_RE_COMPILE_TOKEN_LIMIT.

Thanks!

…ithin duk__match_regexp to prevent CPU usage in systems which use the timeout check for preventing resource exhaustion
@svaarala
Copy link
Owner

svaarala commented May 7, 2020

@braindigitalis Could you rebase to master, it should fix the failing checks?

There are a couple of small things that come to mind from the diff (still worth merging as is):

  • There should be an internal helper to handle throwing a script timeout to reduce code duplication.
  • Clearing the heap interrupt running flag is unnecessary but harmless.
  • The timeout condition is now checked for every step, it might be better to check for every 1024 steps or so, to avoid the macro call (which may be much more expensive than a counter check, e.g. it might read system time).

I can make these adjustments in a follow-up.

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.

2 participants