javadelight / delight-nashorn-sandbox

A sandbox for executing JavaScript with Nashorn in Java.
Other
268 stars 81 forks source link

Issue with Js sanitizer #151

Closed ihevcuk closed 2 months ago

ihevcuk commented 9 months ago

Script fails on js sanitizer with following exception: java.lang.RuntimeException: delight.nashornsandbox.exceptions.ScriptCPUAbuseException: Regular expression running for too many iterations. The operation could NOT be gracefully interrupted.

After that error, even the simplest script will fail with thread interrupted exception. On second call everything is fine.

I'm using java 17 and sandbox version 0.3.2. On version 0.3.0 it works as expected.

mxro commented 9 months ago

Thank you for raising this issue!

Could you provide a code example to help reproduce the issue? Are there any regular expressions in the code that is run by the sandbox?

jpimag commented 6 months ago

Hello, I reproduce the same issue with version 0.4.2 and both jdk 17 or 21. It happens with ugly client script using very long if else statement.

An exemple :

public class NashornSandboxBug {
    public static void main(String[] args) throws ScriptException {
        String s = """
                function(data) {
                  if (data.get("propertyA") == "a special value 1" || data.get("propertyA") == "a special value 2") {
                    return "a special value 1";
                  } else if (data.get("propertyB") == "a special value 3" && (data.get("propertyC") == "a special value 1" || data.get("propertyJ") == "a special value 1" || data.get("propertyV") == "a special value 1")) {
                    return "a special value 1";
                  } else if (data.get("propertyB") == "4" && (data.get("propertyD") == "a special value 1" || data.get("propertyV") == "a special value 1" || data.get("propertyW") == "a special value 1")) {
                    return "a special value 1";
                  } else if (data.get("propertyB") == "a special value 2" && (data.get("propertyE") == "a special value 1" || data.get("propertyF") == "a special value 1" || data.get("propertyL") == "a special value 1")) {
                    return "a special value 1";
                  } else if (data.get("propertyB") == "a special value 3" && (data.get("propertyE") == "a special value 1" || data.get("propertyF") == "a special value 1" || data.get("propertyL") == "a special value 1")) {
                    return "a special value 1";
                  } else if (data.get("propertyB") == "a special value 3" && (data.get("propertyM") == "a special value 1" || data.get("propertyY") == "a special value 1" || data.get("propertyH") == "a special value 1")) {
                    return "a special value 1";
                  } else if (data.get("propertyB") == "a special value 3" && (data.get("propertyM") == "a special value 1" || data.get("propertyY") == "a special value 1" || data.get("propertyH") == "a special value 1")) {
                    return "a special value 1";
                  } else if (data.get("propertyB") == "a special value 3" && (data.get("propertyM") == "a special value 1" || data.get("propertyY") == "a special value 1" || data.get("propertyH") == "a special value 1")) {
                    return "a special value 1";
                  } else if (data.get("propertyB") == "a special value 3" && (data.get("propertyM") == "a special value 1" || data.get("propertyY") == "a special value 1" || data.get("propertyH") == "a special value 1")) {
                    return "a special value 1";
                  } else if (data.get("propertyB") == "a special value 3" && (data.get("propertyM") == "a special value 1" || data.get("propertyY") == "a special value 1" || data.get("propertyH") == "a special value 1")) {
                    return "a special value 1";
                  }  else if (data.get("propertyB") == "a special value 3" && (data.get("propertyM") == "a special value 1" || data.get("propertyY") == "a special value 1" || data.get("propertyH") == "a special value 1")) {
                    return "a special value 1";
                  } else if (data.get("propertyB") == "a special value 3" && (data.get("propertyM") == "a special value 1" || data.get("propertyY") == "a special value 1" || data.get("propertyH") == "a special value 1")) {
                    return "a special value 1";
                  } else {
                     return "0"
                  };

                }
                """;
        NashornSandbox sandbox = NashornSandboxes.create();
        sandbox.eval(s);
    }
}

and the stacktrace :

Exception in thread "main" delight.nashornsandbox.exceptions.ScriptCPUAbuseException: Regular expression running for too many iterations. The operation could NOT be gracefully interrupted.
    at delight.nashornsandbox.internal.SecureInterruptibleCharSequence.charAt(SecureInterruptibleCharSequence.java:23)
    at java.base/java.lang.Character.codePointAt(Character.java:9320)
    at java.base/java.util.regex.Pattern$CharPropertyGreedy.match(Pattern.java:4453)
    at java.base/java.util.regex.Pattern$GroupHead.match(Pattern.java:4969)
    at java.base/java.util.regex.Pattern$Loop.match(Pattern.java:5057)
    at java.base/java.util.regex.Pattern$GroupTail.match(Pattern.java:5000)
    at java.base/java.util.regex.Pattern$BmpCharProperty.match(Pattern.java:4134)
    at java.base/java.util.regex.Pattern$CharPropertyGreedy.match(Pattern.java:4470)
    at java.base/java.util.regex.Pattern$GroupHead.match(Pattern.java:4969)
    at java.base/java.util.regex.Pattern$Loop.match(Pattern.java:5057)
    at java.base/java.util.regex.Pattern$GroupTail.match(Pattern.java:5000)
    at java.base/java.util.regex.Pattern$BmpCharProperty.match(Pattern.java:4134)
    at java.base/java.util.regex.Pattern$CharPropertyGreedy.match(Pattern.java:4470)
    at java.base/java.util.regex.Pattern$GroupHead.match(Pattern.java:4969)
    at java.base/java.util.regex.Pattern$Loop.match(Pattern.java:5057)
    at java.base/java.util.regex.Pattern$GroupTail.match(Pattern.java:5000)
    at java.base/java.util.regex.Pattern$BmpCharProperty.match(Pattern.java:4134)
    at java.base/java.util.regex.Pattern$CharPropertyGreedy.match(Pattern.java:4470)
    at java.base/java.util.regex.Pattern$GroupHead.match(Pattern.java:4969)
    at java.base/java.util.regex.Pattern$Loop.match(Pattern.java:5057)
    at java.base/java.util.regex.Pattern$GroupTail.match(Pattern.java:5000)
    at java.base/java.util.regex.Pattern$BmpCharProperty.match(Pattern.java:4134)
    at java.base/java.util.regex.Pattern$CharPropertyGreedy.match(Pattern.java:4470)
    at java.base/java.util.regex.Pattern$GroupHead.match(Pattern.java:4969)
    at java.base/java.util.regex.Pattern$Loop.match(Pattern.java:5057)
    at java.base/java.util.regex.Pattern$GroupTail.match(Pattern.java:5000)
    at java.base/java.util.regex.Pattern$BmpCharProperty.match(Pattern.java:4134)
    at java.base/java.util.regex.Pattern$CharPropertyGreedy.match(Pattern.java:4470)
    at java.base/java.util.regex.Pattern$GroupHead.match(Pattern.java:4969)
    at java.base/java.util.regex.Pattern$Loop.match(Pattern.java:5057)
    at java.base/java.util.regex.Pattern$GroupTail.match(Pattern.java:5000)
    at java.base/java.util.regex.Pattern$BmpCharProperty.match(Pattern.java:4134)
    at java.base/java.util.regex.Pattern$CharPropertyGreedy.match(Pattern.java:4470)
    at java.base/java.util.regex.Pattern$GroupHead.match(Pattern.java:4969)
    at java.base/java.util.regex.Pattern$Loop.matchInit(Pattern.java:5100)
    at java.base/java.util.regex.Pattern$Prolog.match(Pattern.java:5024)
    at java.base/java.util.regex.Pattern$GroupHead.match(Pattern.java:4969)
    at java.base/java.util.regex.Pattern$StartS.match(Pattern.java:3820)
    at java.base/java.util.regex.Matcher.search(Matcher.java:1767)
    at java.base/java.util.regex.Matcher.find(Matcher.java:787)
    at delight.nashornsandbox.internal.JsSanitizer.injectInterruptionCalls(JsSanitizer.java:236)
    at delight.nashornsandbox.internal.JsSanitizer.secureJsImpl(JsSanitizer.java:284)
    at delight.nashornsandbox.internal.JsSanitizer.secureJs(JsSanitizer.java:263)
    at delight.nashornsandbox.internal.NashornSandboxImpl.eval(NashornSandboxImpl.java:251)
    at delight.nashornsandbox.internal.NashornSandboxImpl.eval(NashornSandboxImpl.java:228)
    at com.eloquant.jp.blackhole.NashornSandboxBug.main(NashornSandboxBug.java:43)

Thanks

rpcai commented 3 months ago

I have the same issue with the 0.4.2 implementation used in https://github.com/thingsboard it appears too be directly related to #139

I am unsure on the mechanics of how Thingsboard uses nashorn-sandbox, but my application code below(I assume part of a string passed to nashorn works:

var variable = {
0:{ status: 'ABCDEF12345', statusName: 'ABCDEF12345' },
1:{ status: 'ABCDEF12345', statusName: 'ABCDEF12345' },
2:{ status: 'ABCDEF12345', statusName: 'ABCDEF12345' },
3:{ status: 'ABCDEF12345', statusName: 'ABCDEF12345' },
4:{ status: 'ABCDEF12345', statusName: 'ABCDEF12345' },
5:{ status: 'ABCDEF12345', statusName: 'ABCDEF12345' },
6:{ status: 'ABCDEF12345', statusName: 'ABCDEF12345' },
7:{ status: 'ABCDEF12345', statusName: 'ABCDEF12345' },
8:{ status: 'ABCDEF12345', statusName: 'ABCDEF12345' },
9:{ status: 'ABCDEF12345', statusName: 'ABCDEF12345' },
10:{ status: 'ABCDEF12345', statusName: 'ABCDEF12345' },
11:{ status: 'ABCDEF12345', statusName: 'ABCDEF12345' },
12:{ status: 'ABCDEF12345', statusName: 'ABCDEF12345' },
13:{ status: 'ABCDEF12345', statusName: 'ABCDEF12345' },
14:{ status: 'ABCDEF12345', statusName: 'ABCDEF12345' },
15:{ status: 'ABCDEF12345', statusName: 'ABCDEF12345' },
16:{ status: 'ABCDEF12345', statusName: 'ABCDEF12345' },
17:{ status: 'ABCDEF', statusName: 'ABCDEF' },
18:{ status: 'ABCDEF', statusName: 'ABCDEF' },
19:{ status: 'ABCDEF', statusName: 'ABCDE' }
};
//This is a Comment
return msg;

adding a single non-whitespace character to the above breaks it.

hkecho commented 2 months ago

Any workaround, met the same issue.

LG987234122 commented 2 months ago

Attempting to parse JavaScript using regular expressions is hard. It might be better to switch to a full JavaScript parser written in Java - as this should be much faster and simpler to insert the necessary JavaScript snippets for checking CPU usage etc. The regular expressions could then be removed.

mxro commented 2 months ago

Thanks for the code examples and further information!

I created a test case with the provided examples:

https://github.com/javadelight/delight-nashorn-sandbox/blob/master/src/test/java/delight/nashornsandbox/TestIssue151RegEx.java#L17

https://github.com/javadelight/delight-nashorn-sandbox/blob/master/src/test/java/delight/nashornsandbox/TestIssue151RegEx.java#L66

Increased the limit of regular expressions allowed in #156 . All tests now pass. Please let me know if there are any further issues.

Released with 0.4.5

busterace commented 2 months ago

Hi @mxro I know you have closed this issue, I checked the changes, you just made the limit of matchCount greater. Due to serious performance problems of Regex, I use AST to transform js and inject code, please have a look at this MR When you are free, thank you. https://github.com/javadelight/delight-nashorn-sandbox/pull/157

mxro commented 2 months ago

Love the changes, that's some great work ❤! Left some comments on the PR and I think a few things to sort out before we can release, but definitely looks like the way forward to me!

busterace commented 2 months ago

@mxro fixed UT performance and npm audit ,pls check, thanks https://github.com/javadelight/delight-nashorn-sandbox/pull/158

mxro commented 1 month ago

Thank you for your great work @busterace ❤

The version with your improvements has been published to Maven central as part of 0.5.0