Skip to content

Commit e63f811

Browse files
aschackmullaschackmull
authored
Merge pull request github#5349 from p0wn4j/fix-nashorn-engine-1
Java: Fix NashornScriptEngine detection in ScriptEngine query
2 parents 84554af + 6841f5f commit e63f811

File tree

14 files changed

+181
-3
lines changed

14 files changed

+181
-3
lines changed

‎java/ql/src/experimental/Security/CWE/CWE-094/NashornScriptEngine.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
// Bad: Execute externally controlled input in Nashorn Script Engine
2+
NashornScriptEngineFactory factory = new NashornScriptEngineFactory();
3+
NashornScriptEngine engine = (NashornScriptEngine) factory.getScriptEngine(new String[] { "-scripting"});
4+
Object result = engine.eval(input);

‎java/ql/src/experimental/Security/CWE/CWE-094/ScriptEngine.qhelp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,12 @@ It allows applications to interact with scripts written in languages such as Jav
1515
<example>
1616
<p>The following code could execute random JavaScript code</p>
1717
<sample src="ScriptEngine.java" />
18+
<sample src="NashornScriptEngine.java" />
1819
</example>
1920

2021
<references>
2122
<li>
2223
CERT coding standard: <a href="https://wiki.sei.cmu.edu/confluence/display/java/IDS52-J.+Prevent+code+injection">ScriptEngine code injection</a>
2324
</li>
2425
</references>
25-
</qhelp>
26+
</qhelp>

‎java/ql/src/experimental/Security/CWE/CWE-094/ScriptEngine.ql

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ import DataFlow::PathGraph
1515

1616
class ScriptEngineMethod extends Method {
1717
ScriptEngineMethod() {
18-
this.getDeclaringType().hasQualifiedName("javax.script", "ScriptEngine") and
18+
this.getDeclaringType().getASupertype*().hasQualifiedName("javax.script", "ScriptEngine") and
1919
this.hasName("eval")
2020
}
2121
}

‎java/ql/test/experimental/query-tests/security/CWE-094/ScriptEngine.expected

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
edges
2+
| ScriptEngineTest.java:8:44:8:55 | input : String | ScriptEngineTest.java:12:37:12:41 | input |
3+
| ScriptEngineTest.java:15:51:15:62 | input : String | ScriptEngineTest.java:19:31:19:35 | input |
4+
| ScriptEngineTest.java:23:58:23:69 | input : String | ScriptEngineTest.java:27:31:27:35 | input |
5+
| ScriptEngineTest.java:30:46:30:57 | input : String | ScriptEngineTest.java:34:31:34:35 | input |
6+
| ScriptEngineTest.java:37:26:37:38 | args : String[] | ScriptEngineTest.java:38:56:38:62 | ...[...] : String |
7+
| ScriptEngineTest.java:37:26:37:38 | args : String[] | ScriptEngineTest.java:39:63:39:69 | ...[...] : String |
8+
| ScriptEngineTest.java:37:26:37:38 | args : String[] | ScriptEngineTest.java:40:70:40:76 | ...[...] : String |
9+
| ScriptEngineTest.java:37:26:37:38 | args : String[] | ScriptEngineTest.java:41:58:41:64 | ...[...] : String |
10+
| ScriptEngineTest.java:38:56:38:62 | ...[...] : String | ScriptEngineTest.java:8:44:8:55 | input : String |
11+
| ScriptEngineTest.java:39:63:39:69 | ...[...] : String | ScriptEngineTest.java:15:51:15:62 | input : String |
12+
| ScriptEngineTest.java:40:70:40:76 | ...[...] : String | ScriptEngineTest.java:23:58:23:69 | input : String |
13+
| ScriptEngineTest.java:41:58:41:64 | ...[...] : String | ScriptEngineTest.java:30:46:30:57 | input : String |
14+
nodes
15+
| ScriptEngineTest.java:8:44:8:55 | input : String | semmle.label | input : String |
16+
| ScriptEngineTest.java:12:37:12:41 | input | semmle.label | input |
17+
| ScriptEngineTest.java:15:51:15:62 | input : String | semmle.label | input : String |
18+
| ScriptEngineTest.java:19:31:19:35 | input | semmle.label | input |
19+
| ScriptEngineTest.java:23:58:23:69 | input : String | semmle.label | input : String |
20+
| ScriptEngineTest.java:27:31:27:35 | input | semmle.label | input |
21+
| ScriptEngineTest.java:30:46:30:57 | input : String | semmle.label | input : String |
22+
| ScriptEngineTest.java:34:31:34:35 | input | semmle.label | input |
23+
| ScriptEngineTest.java:37:26:37:38 | args : String[] | semmle.label | args : String[] |
24+
| ScriptEngineTest.java:38:56:38:62 | ...[...] : String | semmle.label | ...[...] : String |
25+
| ScriptEngineTest.java:39:63:39:69 | ...[...] : String | semmle.label | ...[...] : String |
26+
| ScriptEngineTest.java:40:70:40:76 | ...[...] : String | semmle.label | ...[...] : String |
27+
| ScriptEngineTest.java:41:58:41:64 | ...[...] : String | semmle.label | ...[...] : String |
28+
#select
29+
| ScriptEngineTest.java:12:19:12:42 | eval(...) | ScriptEngineTest.java:37:26:37:38 | args : String[] | ScriptEngineTest.java:12:37:12:41 | input | ScriptEngine eval $@. | ScriptEngineTest.java:37:26:37:38 | args | user input |
30+
| ScriptEngineTest.java:19:19:19:36 | eval(...) | ScriptEngineTest.java:37:26:37:38 | args : String[] | ScriptEngineTest.java:19:31:19:35 | input | ScriptEngine eval $@. | ScriptEngineTest.java:37:26:37:38 | args | user input |
31+
| ScriptEngineTest.java:27:19:27:36 | eval(...) | ScriptEngineTest.java:37:26:37:38 | args : String[] | ScriptEngineTest.java:27:31:27:35 | input | ScriptEngine eval $@. | ScriptEngineTest.java:37:26:37:38 | args | user input |
32+
| ScriptEngineTest.java:34:19:34:36 | eval(...) | ScriptEngineTest.java:37:26:37:38 | args : String[] | ScriptEngineTest.java:34:31:34:35 | input | ScriptEngine eval $@. | ScriptEngineTest.java:37:26:37:38 | args | user input |

‎java/ql/test/experimental/query-tests/security/CWE-094/ScriptEngine.qlref

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
experimental/Security/CWE/CWE-094/ScriptEngine.ql

‎java/ql/test/experimental/query-tests/security/CWE-094/ScriptEngineTest.java

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
import jdk.nashorn.api.scripting.NashornScriptEngine;
2+
import jdk.nashorn.api.scripting.NashornScriptEngineFactory;
3+
import javax.script.*;
4+
5+
6+
public class ScriptEngineTest {
7+
8+
public void testWithScriptEngineReference(String input) throws ScriptException {
9+
ScriptEngineManager scriptEngineManager = new ScriptEngineManager();
10+
// Create with ScriptEngine reference
11+
ScriptEngine scriptEngine = scriptEngineManager.getEngineByExtension("js");
12+
Object result = scriptEngine.eval(input);
13+
}
14+
15+
public void testNashornWithScriptEngineReference(String input) throws ScriptException {
16+
NashornScriptEngineFactory factory = new NashornScriptEngineFactory();
17+
// Create Nashorn with ScriptEngine reference
18+
ScriptEngine engine = (NashornScriptEngine) factory.getScriptEngine(new String[] { "-scripting" });
19+
Object result = engine.eval(input);
20+
}
21+
22+
23+
public void testNashornWithNashornScriptEngineReference(String input) throws ScriptException {
24+
NashornScriptEngineFactory factory = new NashornScriptEngineFactory();
25+
// Create Nashorn with NashornScriptEngine reference
26+
NashornScriptEngine engine = (NashornScriptEngine) factory.getScriptEngine(new String[] { "-scripting" });
27+
Object result = engine.eval(input);
28+
}
29+
30+
public void testCustomScriptEngineReference(String input) throws ScriptException {
31+
MyCustomFactory factory = new MyCustomFactory();
32+
//Create with Custom Script Engine reference
33+
MyCustomScriptEngine engine = (MyCustomScriptEngine) factory.getScriptEngine(new String[] { "-scripting" });
34+
Object result = engine.eval(input);
35+
}
36+
37+
public static void main(String[] args) throws ScriptException {
38+
new ScriptEngineTest().testWithScriptEngineReference(args[0]);
39+
new ScriptEngineTest().testNashornWithScriptEngineReference(args[0]);
40+
new ScriptEngineTest().testNashornWithNashornScriptEngineReference(args[0]);
41+
new ScriptEngineTest().testCustomScriptEngineReference(args[0]);
42+
}
43+
44+
private static class MyCustomScriptEngine extends AbstractScriptEngine {
45+
public Object eval(String var1) throws ScriptException {
46+
return null;
47+
}
48+
}
49+
50+
private static class MyCustomFactory implements ScriptEngineFactory {
51+
public MyCustomFactory() {
52+
}
53+
54+
public ScriptEngine getScriptEngine() { return null; }
55+
56+
public ScriptEngine getScriptEngine(String... args) { return null; }
57+
}
58+
}

‎java/ql/test/experimental/query-tests/security/CWE-094/options

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1,2 @@
1-
//semmle-extractor-options: --javac-args -cp ${testdir}/../../../../stubs/springframework-5.2.3:${testdir}/../../../../stubs/mvel2-2.4.7:${testdir}/../../../../stubs/jsr223-api:${testdir}/../../../../stubs/apache-commons-jexl-2.1.1:${testdir}/../../../../stubs/apache-commons-jexl-3.1
1+
//semmle-extractor-options: --javac-args -cp ${testdir}/../../../../stubs/springframework-5.2.3:${testdir}/../../../../stubs/mvel2-2.4.7:${testdir}/../../../../stubs/jsr223-api:${testdir}/../../../../stubs/apache-commons-jexl-2.1.1:${testdir}/../../../../stubs/apache-commons-jexl-3.1:${testdir}/../../../../stubs/scriptengine
2+

‎java/ql/test/stubs/scriptengine/javax/script/AbstractScriptEngine.java

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
package javax.script;
2+
3+
public abstract class AbstractScriptEngine implements ScriptEngine {
4+
public Object eval(String var1) throws ScriptException {
5+
return null;
6+
}
7+
}
8+

‎java/ql/test/stubs/scriptengine/javax/script/ScriptEngine.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
package javax.script;
2+
3+
public interface ScriptEngine {
4+
Object eval(String var1) throws ScriptException;
5+
}
6+

‎java/ql/test/stubs/scriptengine/javax/script/ScriptEngineFactory.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
package javax.script;
2+
3+
public interface ScriptEngineFactory {
4+
ScriptEngine getScriptEngine();
5+
}
6+

0 commit comments

Comments
 (0)