Skip to content

Commit

Permalink
Addressing #104
Browse files Browse the repository at this point in the history
  • Loading branch information
mxro committed Oct 13, 2020
1 parent ba29e3e commit 77f5882
Show file tree
Hide file tree
Showing 6 changed files with 20 additions and 10 deletions.
5 changes: 5 additions & 0 deletions src/main/java/delight/nashornsandbox/NashornSandbox.java
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,11 @@ public interface NashornSandbox {
/**
* Force, to check if all blocks are enclosed with curly braces "{}".
* <p>
* <b>Warning</b> This option is useful to identify potential abuse but is
* also prone to identify false positives. Please use with caution. Alternatively
* you can use <code>setMaxCPUTime</code> to prevent abusive script execution.
* </p>
* <p>
* Explanation: all loops (for, do-while, while, and if-else, and functions
* should use braces, because poison_pill() function will be inserted after
* each open brace "{", to ensure interruption checking. Otherwise simple
Expand Down
19 changes: 11 additions & 8 deletions src/main/java/delight/nashornsandbox/internal/JsSanitizer.java
Original file line number Diff line number Diff line change
Expand Up @@ -53,11 +53,7 @@ private static class PoisonPil {
private static final List<String> BEAUTIFY_FUNCTIONS = Arrays.asList("exports.beautifier.js;", "window.js_beautify;", "exports.js_beautify;",
"global.js_beautify;");

/** Pattern for back braces. */
private final static List<Pattern> LACK_EXPECTED_BRACES = Arrays.asList(
Pattern.compile("for [^\\{]+$"),
Pattern.compile("^\\s*do [^\\{]*$", Pattern.MULTILINE),
Pattern.compile("^[^\\}]*while [^\\{]+$", Pattern.MULTILINE));


/**
* The name of the JS function to be inserted into user script. To prevent
Expand Down Expand Up @@ -180,7 +176,13 @@ public boolean removeEldestEntry(final Map.Entry<String, String> eldest) {
return new LinkedHashMapSecuredJsCache(linkedHashMap, allowNoBraces);
}

/**
/** Pattern for back braces. */
private final static List<Pattern> LACK_EXPECTED_BRACES = Arrays.asList(
Pattern.compile("for [^\\{]+$"),
Pattern.compile("^\\s*do [^\\{]*$", Pattern.MULTILINE),
Pattern.compile("^[^\\}]*while [^\\{]+$", Pattern.MULTILINE));

/**
* After beautifier every braces should be in place, if not, or too many we need
* to prevent script execution.
*
Expand All @@ -201,6 +203,7 @@ void checkBraces(final String beautifiedJs) throws BracesException {

String line = "";
int index = matcher.start();

while (index >= 0 && withoutComments.charAt(index) != '\n' ) {
line = withoutComments.charAt(index)+line;
index--;
Expand All @@ -213,8 +216,8 @@ void checkBraces(final String beautifiedJs) throws BracesException {
// for in string

} else {
throw new BracesException("No block braces after function|for|while|do. Found ["+matcher.group()+"]. "+
"If this exception is reported in error, please set the option `allowNoBraces(true)`");
throw new BracesException("Potentially no block braces after function|for|while|do. Found ["+matcher.group()+"]. "+
"To disable this exception, please set the option `allowNoBraces(true)`");
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ public class NashornSandboxImpl implements NashornSandbox {

protected boolean allowGlobalsObjects = false;

protected boolean allowNoBraces = false;
protected boolean allowNoBraces = true;

protected JsEvaluator evaluator;

Expand Down
1 change: 0 additions & 1 deletion src/test/java/delight/nashornsandbox/TestIssue102.java
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ public void test() throws ScriptCPUAbuseException, ScriptException, NoSuchMethod
sandbox.eval(function);
Invocable invocable = sandbox.getSandboxedInvocable();
Object result = invocable.invokeFunction("execute");
System.out.println(result);
Assert.assertNotNull(result);
}

Expand Down
2 changes: 2 additions & 0 deletions src/test/java/delight/nashornsandbox/TestLimitCPU.java
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ public void test_nice_script() throws ScriptCPUAbuseException, ScriptException {
@Test(expected = BracesException.class)
public void test_only_while() throws ScriptCPUAbuseException, ScriptException {
final NashornSandbox sandbox = NashornSandboxes.create();
sandbox.allowNoBraces(false);
sandbox.setMaxCPUTime(50);
sandbox.setExecutor(Executors.newSingleThreadExecutor());
try {
Expand Down Expand Up @@ -117,6 +118,7 @@ public void test_only_while_good_script() throws ScriptCPUAbuseException, Script
@Test(expected = BracesException.class)
public void test_while_plus_iteration_bad_script() throws ScriptCPUAbuseException, ScriptException {
final NashornSandbox sandbox = NashornSandboxes.create();
sandbox.allowNoBraces(false);
try {
sandbox.setMaxCPUTime(50);
sandbox.setExecutor(Executors.newSingleThreadExecutor());
Expand Down
1 change: 1 addition & 0 deletions src/test/java/delight/nashornsandbox/TestMemoryLimit.java
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ public void test() throws ScriptCPUAbuseException, ScriptException {
@Test(expected = BracesException.class)
public void test_noexpectedbraces() throws ScriptCPUAbuseException, ScriptException {
final NashornSandbox sandbox = NashornSandboxes.create();
sandbox.allowNoBraces(false);
try {
sandbox.setMaxMemory(MEMORY_LIMIT);
sandbox.setExecutor(Executors.newSingleThreadExecutor());
Expand Down

0 comments on commit 77f5882

Please sign in to comment.