Skip to content

Commit 2be68b2

Browse files
Apply suggestions from code review
Co-authored-by: Tony Torralba <[email protected]>
1 parent a14ebb7 commit 2be68b2

File tree

1 file changed

+16
-21
lines changed

1 file changed

+16
-21
lines changed

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

Lines changed: 16 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33
import java
44
import semmle.code.xml.AndroidManifest
55
import semmle.code.java.dataflow.TaintTracking
6+
import semmle.code.java.frameworks.Networking
7+
import semmle.code.java.security.Encryption
68
import HttpsUrls
79

810
/** An Android Network Security Configuration XML file. */
@@ -58,21 +60,17 @@ predicate trustedDomain(string domainName) {
5860
* `default` is true if the default SSL socket factory for all URLs is being set.
5961
*/
6062
private predicate trustedSocketFactory(MethodAccess setSocketFactory, boolean default) {
61-
exists(MethodAccess getSocketFactory, MethodAccess initSslContext, string methodName |
62-
setSocketFactory
63-
.getMethod()
64-
.getASourceOverriddenMethod*()
65-
.hasQualifiedName("javax.net.ssl", "HttpsURLConnection", methodName) and
66-
(
67-
default = true and methodName = "setDefaultSSLSocketFactory"
63+
exists(MethodAccess getSocketFactory, MethodAccess initSslContext |
64+
exists(Method m | setSocketFactory.getMethod().getASourceOverriddenMethod*() = m |
65+
default = true and
66+
m.getDeclaringType() instanceof HttpsUrlConnection and
67+
m.hasName("setDefaultSSLSocketFactory")
6868
or
69-
default = false and methodName = "setSSLSocketFactory"
69+
default = false and m instanceof SetConnectionFactoryMethod
7070
) and
71-
initSslContext.getMethod().hasQualifiedName("javax.net.ssl", "SSLContext", "init") and
72-
getSocketFactory
73-
.getMethod()
74-
.getASourceOverriddenMethod*()
75-
.hasQualifiedName("javax.net.ssl", "SSLContext", "getSocketFactory") and
71+
initSslContext.getMethod().getDeclaringType() instanceof SslContext and
72+
initSslContext.getMethod().hasName("init") and
73+
getSocketFactory.getMethod().getASourceOverriddenMethod*() instanceof GetSocketFactory and
7674
not initSslContext.getArgument(1) instanceof NullLiteral and
7775
DataFlow::localExprFlow(initSslContext.getQualifier(), getSocketFactory.getQualifier()) and
7876
DataFlow::localExprFlow(getSocketFactory, setSocketFactory.getArgument(0))
@@ -85,10 +83,7 @@ private predicate trustedSocketFactory(MethodAccess setSocketFactory, boolean de
8583
*/
8684
private predicate trustedUrlConnection(Expr url) {
8785
exists(MethodAccess openCon |
88-
openCon
89-
.getMethod()
90-
.getASourceOverriddenMethod*()
91-
.hasQualifiedName("java.net", "URL", "openConnection") and
86+
openCon.getMethod().getASourceOverriddenMethod*() instanceof UrlOpenConnectionMethod and
9287
url = openCon.getQualifier() and
9388
exists(MethodAccess setSocketFactory |
9489
trustedSocketFactory(setSocketFactory, false) and
@@ -97,10 +92,10 @@ private predicate trustedUrlConnection(Expr url) {
9792
)
9893
or
9994
trustedSocketFactory(_, true) and
100-
exists(MethodAccess open |
101-
open.getMethod()
102-
.getASourceOverriddenMethod*()
103-
.hasQualifiedName("java.net", "URL", ["openConnection", "openStream"]) and
95+
exists(MethodAccess open, Method m |
96+
m instanceof UrlOpenConnectionMethod or m instanceof UrlOpenStreamMethod
97+
|
98+
open.getMethod().getASourceOverriddenMethod*() = m and
10499
url = open.getQualifier()
105100
)
106101
}

0 commit comments

Comments
 (0)