hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2026-05-01 11:45:14 +02:00
Code Issues Packages Projects Releases Wiki Activity

Fix the case where user-controlled input is passed as URL to env Hashtable

Browse Source
This commit is contained in:
Grzegorz Golawski
2020-05-08 00:44:22 +02:00
parent df9921f870
commit afea9330b7
3 changed files with 57 additions and 50 deletions
Download Patch File Download Diff File Expand all files Collapse all files

63
java/ql/src/experimental/Security/CWE/CWE-074/JndiInjectionLib.qll
View File

@@ -24,11 +24,15 @@ class JndiInjectionFlowConfig extends TaintTracking::Configuration {
nameStep(node1, node2) or
jmxServiceUrlStep(node1, node2) or
jmxConnectorStep(node1, node2) or
rmiConnectorStep(node1, node2) or
providerUrlEnvStep(node1, node2)
rmiConnectorStep(node1, node2)
}
}
/** The class `java.util.Hashtable`. */
class TypeHashtable extends Class {
TypeHashtable() { this.getSourceDeclaration().hasQualifiedName("java.util", "Hashtable") }
}
/** The class `javax.naming.directory.SearchControls`. */
class TypeSearchControls extends Class {
TypeSearchControls() { this.hasQualifiedName("javax.naming.directory", "SearchControls") }
@@ -100,7 +104,7 @@ predicate jndiSinkMethod(Method m, int index) {
*/
predicate springJndiTemplateSinkMethod(Method m, int index) {
m.getDeclaringType() instanceof TypeSpringJndiTemplate and
(m.hasName("lookup") or m.hasName("setEnvironment")) and
m.hasName("lookup") and
index = 0
}
@@ -155,13 +159,33 @@ predicate jmxConnectorFactorySinkMethod(Method m, int index) {
index = 0
}
/**
* Tainted value passed to env `Hashtable` as the prodiver UDL, i.e.
* `env.put(Context.PROVIDER_URL, tainted)` or `env.setProperty(Context.PROVIDER_URL, tainted)`.
*/
predicate providerUrlEnv(MethodAccess ma, Method m, int index) {
m.getDeclaringType().getAnAncestor() instanceof TypeHashtable and
(m.hasName("put") or m.hasName("setProperty")) and
(
ma.getArgument(0).(CompileTimeConstantExpr).getStringValue() = "java.naming.provider.url"
or
exists(Field f |
ma.getArgument(0) = f.getAnAccess() and
f.hasName("PROVIDER_URL") and
f.getDeclaringType() instanceof TypeNamingContext
)
) and
index = 1
}
/** Holds if parameter at index `index` in method `m` is JNDI injection sink. */
predicate jndiInjectionSinkMethod(MethodAccess ma, Method m, int index) {
jndiSinkMethod(m, index) or
springJndiTemplateSinkMethod(m, index) or
springLdapTemplateSinkMethod(ma, m, index) or
shiroSinkMethod(m, index) or
jmxConnectorFactorySinkMethod(m, index)
jmxConnectorFactorySinkMethod(m, index) or
providerUrlEnv(ma, m, index)
}
/** A data flow sink for unvalidated user input that is used in JNDI lookup. */
@@ -179,13 +203,6 @@ class JndiInjectionSink extends DataFlow::ExprNode {
m.getDeclaringType().getAnAncestor() instanceof TypeJMXConnector and
m.hasName("connect")
)
or
exists(ConstructorCall cc |
cc.getConstructedType().getAnAncestor() instanceof TypeInitialContext or
cc.getConstructedType() instanceof TypeSpringJndiTemplate
|
cc.getArgument(0) = this.getExpr()
)
}
}
@@ -236,27 +253,3 @@ predicate rmiConnectorStep(ExprNode n1, ExprNode n2) {
n2.asExpr() = cc
)
}
/**
* Holds if `n1` to `n2` is a dataflow step that converts between `String` and
* `Hashtable` or `Properties`, i.e. `env.put(Context.PROVIDER_URL, tainted)` or
* `env.setProperty(Context.PROVIDER_URL, tainted)`.
*/
predicate providerUrlEnvStep(ExprNode n1, ExprNode n2) {
exists(MethodAccess ma, Method m |
n1.asExpr() = ma.getArgument(1) and n2.asExpr() = ma.getQualifier()
|
ma.getMethod() = m and
m.getDeclaringType().getAnAncestor() instanceof TypeProperty and
(m.hasName("put") or m.hasName("setProperty")) and
(
ma.getArgument(0).(CompileTimeConstantExpr).getStringValue() = "java.naming.provider.url"
or
exists(Field f |
ma.getArgument(0) = f.getAnAccess() and
f.hasName("PROVIDER_URL") and
f.getDeclaringType() instanceof TypeNamingContext
)
)
)
}

30
java/ql/test/experimental/query-tests/security/CWE-074/JndiInjection.expected
View File

@@ -49,11 +49,11 @@ edges
| JndiInjection.java:106:41:106:68 | nameStr : String | JndiInjection.java:110:16:110:22 | nameStr |
| JndiInjection.java:113:37:113:63 | urlStr : String | JndiInjection.java:114:33:114:57 | new JMXServiceURL(...) |
| JndiInjection.java:113:37:113:63 | urlStr : String | JndiInjection.java:118:5:118:13 | connector |
| JndiInjection.java:121:27:121:53 | urlStr : String | JndiInjection.java:125:24:125:26 | env |
| JndiInjection.java:128:27:128:53 | urlStr : String | JndiInjection.java:132:27:132:29 | env |
| JndiInjection.java:135:52:135:78 | urlStr : String | JndiInjection.java:139:22:139:26 | props |
| JndiInjection.java:142:52:142:78 | urlStr : String | JndiInjection.java:146:22:146:26 | props |
| JndiInjection.java:149:52:149:78 | urlStr : String | JndiInjection.java:154:29:154:33 | props |
| JndiInjection.java:121:27:121:53 | urlStr : String | JndiInjection.java:124:35:124:40 | urlStr |
| JndiInjection.java:128:27:128:53 | urlStr : String | JndiInjection.java:131:41:131:46 | urlStr |
| JndiInjection.java:135:52:135:78 | urlStr : String | JndiInjection.java:138:37:138:42 | urlStr |
| JndiInjection.java:142:52:142:78 | urlStr : String | JndiInjection.java:145:51:145:56 | urlStr |
| JndiInjection.java:149:52:149:78 | urlStr : String | JndiInjection.java:152:51:152:56 | urlStr |
nodes
| JndiInjection.java:26:38:26:65 | nameStr : String | semmle.label | nameStr : String |
| JndiInjection.java:30:16:30:22 | nameStr | semmle.label | nameStr |
@@ -113,15 +113,15 @@ nodes
| JndiInjection.java:114:33:114:57 | new JMXServiceURL(...) | semmle.label | new JMXServiceURL(...) |
| JndiInjection.java:118:5:118:13 | connector | semmle.label | connector |
| JndiInjection.java:121:27:121:53 | urlStr : String | semmle.label | urlStr : String |
| JndiInjection.java:125:24:125:26 | env | semmle.label | env |
| JndiInjection.java:124:35:124:40 | urlStr | semmle.label | urlStr |
| JndiInjection.java:128:27:128:53 | urlStr : String | semmle.label | urlStr : String |
| JndiInjection.java:132:27:132:29 | env | semmle.label | env |
| JndiInjection.java:131:41:131:46 | urlStr | semmle.label | urlStr |
| JndiInjection.java:135:52:135:78 | urlStr : String | semmle.label | urlStr : String |
| JndiInjection.java:139:22:139:26 | props | semmle.label | props |
| JndiInjection.java:138:37:138:42 | urlStr | semmle.label | urlStr |
| JndiInjection.java:142:52:142:78 | urlStr : String | semmle.label | urlStr : String |
| JndiInjection.java:146:22:146:26 | props | semmle.label | props |
| JndiInjection.java:145:51:145:56 | urlStr | semmle.label | urlStr |
| JndiInjection.java:149:52:149:78 | urlStr : String | semmle.label | urlStr : String |
| JndiInjection.java:154:29:154:33 | props | semmle.label | props |
| JndiInjection.java:152:51:152:56 | urlStr | semmle.label | urlStr |
#select
| JndiInjection.java:30:16:30:22 | nameStr | JndiInjection.java:26:38:26:65 | nameStr : String | JndiInjection.java:30:16:30:22 | nameStr | JNDI lookup might include name from $@. | JndiInjection.java:26:38:26:65 | nameStr | this user input |
| JndiInjection.java:31:20:31:26 | nameStr | JndiInjection.java:26:38:26:65 | nameStr : String | JndiInjection.java:31:20:31:26 | nameStr | JNDI lookup might include name from $@. | JndiInjection.java:26:38:26:65 | nameStr | this user input |
@@ -173,8 +173,8 @@ nodes
| JndiInjection.java:110:16:110:22 | nameStr | JndiInjection.java:106:41:106:68 | nameStr : String | JndiInjection.java:110:16:110:22 | nameStr | JNDI lookup might include name from $@. | JndiInjection.java:106:41:106:68 | nameStr | this user input |
| JndiInjection.java:114:33:114:57 | new JMXServiceURL(...) | JndiInjection.java:113:37:113:63 | urlStr : String | JndiInjection.java:114:33:114:57 | new JMXServiceURL(...) | JNDI lookup might include name from $@. | JndiInjection.java:113:37:113:63 | urlStr | this user input |
| JndiInjection.java:118:5:118:13 | connector | JndiInjection.java:113:37:113:63 | urlStr : String | JndiInjection.java:118:5:118:13 | connector | JNDI lookup might include name from $@. | JndiInjection.java:113:37:113:63 | urlStr | this user input |
| JndiInjection.java:125:24:125:26 | env | JndiInjection.java:121:27:121:53 | urlStr : String | JndiInjection.java:125:24:125:26 | env | JNDI lookup might include name from $@. | JndiInjection.java:121:27:121:53 | urlStr | this user input |
| JndiInjection.java:132:27:132:29 | env | JndiInjection.java:128:27:128:53 | urlStr : String | JndiInjection.java:132:27:132:29 | env | JNDI lookup might include name from $@. | JndiInjection.java:128:27:128:53 | urlStr | this user input |
| JndiInjection.java:139:22:139:26 | props | JndiInjection.java:135:52:135:78 | urlStr : String | JndiInjection.java:139:22:139:26 | props | JNDI lookup might include name from $@. | JndiInjection.java:135:52:135:78 | urlStr | this user input |
| JndiInjection.java:146:22:146:26 | props | JndiInjection.java:142:52:142:78 | urlStr : String | JndiInjection.java:146:22:146:26 | props | JNDI lookup might include name from $@. | JndiInjection.java:142:52:142:78 | urlStr | this user input |
| JndiInjection.java:154:29:154:33 | props | JndiInjection.java:149:52:149:78 | urlStr : String | JndiInjection.java:154:29:154:33 | props | JNDI lookup might include name from $@. | JndiInjection.java:149:52:149:78 | urlStr | this user input |
| JndiInjection.java:124:35:124:40 | urlStr | JndiInjection.java:121:27:121:53 | urlStr : String | JndiInjection.java:124:35:124:40 | urlStr | JNDI lookup might include name from $@. | JndiInjection.java:121:27:121:53 | urlStr | this user input |
| JndiInjection.java:131:41:131:46 | urlStr | JndiInjection.java:128:27:128:53 | urlStr : String | JndiInjection.java:131:41:131:46 | urlStr | JNDI lookup might include name from $@. | JndiInjection.java:128:27:128:53 | urlStr | this user input |
| JndiInjection.java:138:37:138:42 | urlStr | JndiInjection.java:135:52:135:78 | urlStr : String | JndiInjection.java:138:37:138:42 | urlStr | JNDI lookup might include name from $@. | JndiInjection.java:135:52:135:78 | urlStr | this user input |
| JndiInjection.java:145:51:145:56 | urlStr | JndiInjection.java:142:52:142:78 | urlStr : String | JndiInjection.java:145:51:145:56 | urlStr | JNDI lookup might include name from $@. | JndiInjection.java:142:52:142:78 | urlStr | this user input |
| JndiInjection.java:152:51:152:56 | urlStr | JndiInjection.java:149:52:149:78 | urlStr : String | JndiInjection.java:152:51:152:56 | urlStr | JNDI lookup might include name from $@. | JndiInjection.java:149:52:149:78 | urlStr | this user input |

14
java/ql/test/experimental/query-tests/security/CWE-074/JndiInjection.java
View File

@@ -174,4 +174,18 @@ public class JndiInjection {
ctx.searchForObject(nameStr, "", new SearchControls(), (ContextMapper) new Object());
}
public void testEnvOk1(@RequestParam String urlStr) throws NamingException {
Hashtable<String, String> env = new Hashtable<String, String>();
env.put(Context.INITIAL_CONTEXT_FACTORY, "com.sun.jndi.rmi.registry.RegistryContextFactory");
env.put(Context.SECURITY_PRINCIPAL, urlStr);
new InitialContext(env);
}
public void testEnvOk2(@RequestParam String urlStr) throws NamingException {
Hashtable<String, String> env = new Hashtable<String, String>();
env.put(Context.INITIAL_CONTEXT_FACTORY, "com.sun.jndi.rmi.registry.RegistryContextFactory");
env.put("java.naming.security.principal", urlStr);
new InitialContext(env);
}
}