Skip to content

Commit

Permalink
Protect against RegEx attacks in santising script input (#139)
Browse files Browse the repository at this point in the history
* Adding test to assert system environment

* Setting Java max memory for tests

* Fixing java max memory allocations

* Fixing upper memory limit

* Adding comment to unit test

* Adding protecting against regex attack

* Increasing max operations on regex
  • Loading branch information
mxro committed May 26, 2023
1 parent 79e9ea8 commit b899b8e
Show file tree
Hide file tree
Showing 5 changed files with 101 additions and 4 deletions.
1 change: 1 addition & 0 deletions .classpath
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
<attributes>
<attribute name="maven.pomderived" value="true"/>
<attribute name="test" value="true"/>
<attribute name="optional" value="true"/>
</attributes>
</classpathentry>
<classpathentry kind="src" path="target/generated-sources/annotations">
Expand Down
6 changes: 6 additions & 0 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,12 @@
<version>1.7.25</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>com.github.mifmif</groupId>
<artifactId>generex</artifactId>
<version>1.0.2</version>
<scope>test</scope>
</dependency>
</dependencies>

<!-- XXXXXXXXXXXXXX Maven declarations XXXXXXXXXXXXXXXXXX -->
Expand Down
11 changes: 7 additions & 4 deletions src/main/java/delight/nashornsandbox/internal/JsSanitizer.java
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ private static class PoisonPil {
this.allowNoBraces = allowBraces;
this.securedJsCache = createSecuredJsCache(maxPreparedStatements);
assertScriptEngine();
Object beautifHandler = getBeautifHandler(scriptEngine);
Object beautifHandler = getBeautifyHandler(scriptEngine);
this.jsBeautify = beautifierAsFunction(beautifHandler);
}

Expand All @@ -124,7 +124,7 @@ private static class PoisonPil {
this.allowNoBraces = allowBraces;
this.securedJsCache = cache;
assertScriptEngine();
Object beautifHandler = getBeautifHandler(scriptEngine);
Object beautifHandler = getBeautifyHandler(scriptEngine);
this.jsBeautify = beautifierAsFunction(beautifHandler);
}

Expand All @@ -141,7 +141,7 @@ private void assertScriptEngine() {
}
}

private static Object getBeautifHandler(final ScriptEngine scriptEngine) {
private static Object getBeautifyHandler(final ScriptEngine scriptEngine) {
try {
for (final String name : BEAUTIFY_FUNCTIONS) {
final Object somWindow = scriptEngine.eval(name);
Expand Down Expand Up @@ -229,7 +229,10 @@ String injectInterruptionCalls(final String str) {
String current = str;
for (final PoisonPil pp : POISON_PILLS) {
final StringBuffer sb = new StringBuffer();
final Matcher matcher = pp.pattern.matcher(current);
final ThreadLocal<Integer> matchCount = new ThreadLocal<>();
matchCount.set(0);
final Matcher matcher = pp.pattern.matcher(new SecureInterruptibleCharSequence(current,
matchCount));
while (matcher.find()) {
matcher.appendReplacement(sb, ("$1" + pp.replacement));
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
package delight.nashornsandbox.internal;

import delight.nashornsandbox.exceptions.ScriptCPUAbuseException;

public class SecureInterruptibleCharSequence implements CharSequence {
final CharSequence inner;
final ThreadLocal<Integer> matchCount;

public SecureInterruptibleCharSequence(CharSequence inner, ThreadLocal<Integer> matchCount) {
super();
this.inner = inner;
this.matchCount = matchCount;
}

@Override
public char charAt(int index) {
matchCount.set(matchCount.get()+1);
if (matchCount.get() > 5000000){
Thread thread = Thread.currentThread();
thread.interrupt();
}
if (Thread.currentThread().isInterrupted()) {
throw new ScriptCPUAbuseException("Regular expression running for too many iterations.", true, null);
}
return inner.charAt(index);
}

@Override
public int length() {
return inner.length();
}

@Override
public CharSequence subSequence(int start, int end) {
return new SecureInterruptibleCharSequence(inner.subSequence(start, end), matchCount);
}

@Override
public String toString() {
return inner.toString();
}
}
45 changes: 45 additions & 0 deletions src/test/java/delight/nashornsandbox/TestIssue117.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
package delight.nashornsandbox;

import javax.script.ScriptException;

import org.junit.Assert;
import org.junit.Test;

import com.mifmif.common.regex.Generex;

import delight.nashornsandbox.exceptions.ScriptCPUAbuseException;

public class TestIssue117 {

public static String getMatchStr(String regex, int repl) {
Generex generex = new Generex(regex);

String randomStr = generex.random();
StringBuffer sb = new StringBuffer();
sb.append(randomStr);
sb.append(";\n");
for (int i = 0; i < repl; i++) {
sb.append("/");
}

return sb.toString();
}

@Test
public void test() throws ScriptCPUAbuseException, ScriptException, NoSuchMethodException {

NashornSandbox sandbox = NashornSandboxes.create();

for (int i = 480; i <= 500; i++) {
long startTime = System.currentTimeMillis();
String js_script = getMatchStr("(([^;]+;){9}[^;]+)", i);
try {
sandbox.eval(js_script);
} catch (Exception e) {
}
long endTime = System.currentTimeMillis();
long costTime = endTime - startTime;
Assert.assertTrue("RegEx attack successful. Took longer than 5000 ms to resolve script. Time required: "+costTime, costTime <= 5000);
}
}
}

0 comments on commit b899b8e

Please sign in to comment.