Skip to content

Commit fea260b

Browse files
committed
Java: Diff-informed UnsafeHostnameVerification.ql
This commit also adds a test case that would fail under `codeql test run --check-diff-informed` if not for the override of `getASelectedSourceLocation`. There was no existing such test since all the existing tests used anonymous classes whose location was on the same line as the source.
1 parent 8224ef6 commit fea260b

File tree

3 files changed

+29
-0
lines changed

3 files changed

+29
-0
lines changed

java/ql/lib/semmle/code/java/security/UnsafeHostnameVerificationQuery.qll

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,17 @@ module TrustAllHostnameVerifierConfig implements DataFlow::ConfigSig {
6565
"|(set)?(accept|trust|ignore|allow)(all|every|any)" +
6666
"|(use|do|enable)insecure|(set|do|use)?no.*(check|validation|verify|verification)|disable).*$")
6767
}
68+
69+
predicate observeDiffInformedIncrementalMode() { any() }
70+
71+
Location getASelectedSourceLocation(DataFlow::Node source) {
72+
isSource(source) and
73+
(
74+
result = source.getLocation()
75+
or
76+
result = source.asExpr().(ClassInstanceExpr).getConstructedType().getLocation()
77+
)
78+
}
6879
}
6980

7081
/** Data flow to model the flow of a `TrustAllHostnameVerifier` to a `set(Default)HostnameVerifier` call. */

java/ql/test/query-tests/security/CWE-297/UnsafeHostnameVerification.expected

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
| UnsafeHostnameVerification.java:47:55:47:71 | ...->... | UnsafeHostnameVerification.java:47:55:47:71 | ...->... | UnsafeHostnameVerification.java:47:55:47:71 | ...->... | The $@ defined by $@ always accepts any certificate, even if the hostname does not match. | UnsafeHostnameVerification.java:47:55:47:71 | ...->... | hostname verifier | UnsafeHostnameVerification.java:47:55:47:71 | new HostnameVerifier(...) { ... } | this type |
55
| UnsafeHostnameVerification.java:81:55:81:62 | verifier | UnsafeHostnameVerification.java:66:37:80:9 | new (...) : new HostnameVerifier(...) { ... } | UnsafeHostnameVerification.java:81:55:81:62 | verifier | The $@ defined by $@ always accepts any certificate, even if the hostname does not match. | UnsafeHostnameVerification.java:66:37:80:9 | new (...) : new HostnameVerifier(...) { ... } | hostname verifier | UnsafeHostnameVerification.java:66:41:66:56 | new HostnameVerifier(...) { ... } | this type |
66
| UnsafeHostnameVerification.java:94:55:94:62 | verifier | UnsafeHostnameVerification.java:88:37:93:9 | new (...) : new HostnameVerifier(...) { ... } | UnsafeHostnameVerification.java:94:55:94:62 | verifier | The $@ defined by $@ always accepts any certificate, even if the hostname does not match. | UnsafeHostnameVerification.java:88:37:93:9 | new (...) : new HostnameVerifier(...) { ... } | hostname verifier | UnsafeHostnameVerification.java:88:41:88:56 | new HostnameVerifier(...) { ... } | this type |
7+
| UnsafeHostnameVerification.java:116:55:116:78 | new AlwaysTrueVerifier(...) | UnsafeHostnameVerification.java:116:55:116:78 | new AlwaysTrueVerifier(...) | UnsafeHostnameVerification.java:116:55:116:78 | new AlwaysTrueVerifier(...) | The $@ defined by $@ always accepts any certificate, even if the hostname does not match. | UnsafeHostnameVerification.java:116:55:116:78 | new AlwaysTrueVerifier(...) | hostname verifier | UnsafeHostnameVerification.java:104:26:104:43 | AlwaysTrueVerifier | this type |
78
edges
89
| UnsafeHostnameVerification.java:66:37:80:9 | new (...) : new HostnameVerifier(...) { ... } | UnsafeHostnameVerification.java:81:55:81:62 | verifier | provenance | Sink:MaD:1 |
910
| UnsafeHostnameVerification.java:88:37:93:9 | new (...) : new HostnameVerifier(...) { ... } | UnsafeHostnameVerification.java:94:55:94:62 | verifier | provenance | Sink:MaD:1 |
@@ -23,4 +24,5 @@ nodes
2324
| UnsafeHostnameVerification.java:94:55:94:62 | verifier | semmle.label | verifier |
2425
| UnsafeHostnameVerification.java:97:42:97:68 | ALLOW_ALL_HOSTNAME_VERIFIER : new HostnameVerifier(...) { ... } | semmle.label | ALLOW_ALL_HOSTNAME_VERIFIER : new HostnameVerifier(...) { ... } |
2526
| UnsafeHostnameVerification.java:97:72:102:5 | new (...) : new HostnameVerifier(...) { ... } | semmle.label | new (...) : new HostnameVerifier(...) { ... } |
27+
| UnsafeHostnameVerification.java:116:55:116:78 | new AlwaysTrueVerifier(...) | semmle.label | new AlwaysTrueVerifier(...) |
2628
subpaths

java/ql/test/query-tests/security/CWE-297/UnsafeHostnameVerification.java

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,4 +100,20 @@ public boolean verify(String hostname, SSLSession session) {
100100
return true; // BAD, always returns true
101101
}
102102
};
103+
104+
private static class AlwaysTrueVerifier implements HostnameVerifier {
105+
@Override
106+
public boolean verify(String hostname, SSLSession session) {
107+
return true; // BAD, always returns true
108+
}
109+
}
110+
111+
/**
112+
* Same as testTrustAllHostnameOfAnonymousClass, but with a named class.
113+
* This is for testing the diff-informed functionality of the query.
114+
*/
115+
public void testTrustAllHostnameOfNamedClass() {
116+
HttpsURLConnection.setDefaultHostnameVerifier(new AlwaysTrueVerifier());
117+
}
118+
103119
}

0 commit comments

Comments
 (0)