Skip to content

Commit 8d8126f

Browse files
MalaykeMalayke
authored
Merge branch 'github:main' into main
2 parents ac465b9 + 8ed9fbb commit 8d8126f

File tree

17 files changed

+187
-58
lines changed

17 files changed

+187
-58
lines changed

‎cpp/ql/src/Critical/FlowAfterFree.qll

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,8 @@ module FlowFromFree<isSinkSig/2 isASink, isExcludedSig/2 isExcluded> {
8787
|
8888
e = any(StoreInstruction store).getDestinationAddress().getUnconvertedResultExpression()
8989
)
90+
or
91+
n.asExpr() instanceof ArrayExpr
9092
}
9193
}
9294

‎cpp/ql/src/jsf/4.16 Initialization/AV Rule 145.ql

Lines changed: 30 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -32,18 +32,41 @@ predicate hasReferenceInitializer(EnumConstant c) {
3232
)
3333
}
3434

35+
/**
36+
* Gets the `rnk`'th (1-based) enumeration constant in `e` that does not have a
37+
* reference initializer (i.e., an initializer that refers to an enumeration
38+
* constant from the same enumeration).
39+
*/
40+
EnumConstant getNonReferenceInitializedEnumConstantByRank(Enum e, int rnk) {
41+
result =
42+
rank[rnk](EnumConstant cand, int pos, string filepath, int startline, int startcolumn |
43+
e.getEnumConstant(pos) = cand and
44+
not hasReferenceInitializer(cand) and
45+
cand.getLocation().hasLocationInfo(filepath, startline, startcolumn, _, _)
46+
|
47+
cand order by pos, filepath, startline, startcolumn
48+
)
49+
}
50+
51+
/**
52+
* Holds if `ec` is not the last enumeration constant in `e` that has a non-
53+
* reference initializer.
54+
*/
55+
predicate hasNextWithoutReferenceInitializer(Enum e, EnumConstant ec) {
56+
exists(int rnk |
57+
ec = getNonReferenceInitializedEnumConstantByRank(e, rnk) and
58+
exists(getNonReferenceInitializedEnumConstantByRank(e, rnk + 1))
59+
)
60+
}
61+
3562
// There exists another constant whose value is implicit, but it's
3663
// not the last one: the last value is okay to use to get the highest
3764
// enum value automatically. It can be followed by aliases though.
3865
predicate enumThatHasConstantWithImplicitValue(Enum e) {
39-
exists(EnumConstant ec, int pos |
40-
ec = e.getEnumConstant(pos) and
66+
exists(EnumConstant ec |
67+
ec = e.getAnEnumConstant() and
4168
not hasInitializer(ec) and
42-
exists(EnumConstant ec2, int pos2 |
43-
ec2 = e.getEnumConstant(pos2) and
44-
pos2 > pos and
45-
not hasReferenceInitializer(ec2)
46-
)
69+
hasNextWithoutReferenceInitializer(e, ec)
4770
)
4871
}
4972

‎cpp/ql/test/query-tests/Critical/MemoryFreed/DoubleFree.expected

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ edges
99
| test_free.cpp:83:12:83:12 | pointer to operator delete output argument | test_free.cpp:85:12:85:12 | a |
1010
| test_free.cpp:101:10:101:10 | pointer to free output argument | test_free.cpp:103:10:103:10 | a |
1111
| test_free.cpp:128:10:128:11 | pointer to free output argument | test_free.cpp:129:10:129:11 | * ... |
12-
| test_free.cpp:131:10:131:13 | pointer to free output argument | test_free.cpp:132:10:132:13 | access to array |
1312
| test_free.cpp:152:27:152:27 | pointer to free output argument | test_free.cpp:154:10:154:10 | a |
1413
| test_free.cpp:207:10:207:10 | pointer to free output argument | test_free.cpp:209:10:209:10 | a |
1514
nodes
@@ -33,8 +32,6 @@ nodes
3332
| test_free.cpp:103:10:103:10 | a | semmle.label | a |
3433
| test_free.cpp:128:10:128:11 | pointer to free output argument | semmle.label | pointer to free output argument |
3534
| test_free.cpp:129:10:129:11 | * ... | semmle.label | * ... |
36-
| test_free.cpp:131:10:131:13 | pointer to free output argument | semmle.label | pointer to free output argument |
37-
| test_free.cpp:132:10:132:13 | access to array | semmle.label | access to array |
3835
| test_free.cpp:152:27:152:27 | pointer to free output argument | semmle.label | pointer to free output argument |
3936
| test_free.cpp:154:10:154:10 | a | semmle.label | a |
4037
| test_free.cpp:207:10:207:10 | pointer to free output argument | semmle.label | pointer to free output argument |
@@ -51,6 +48,5 @@ subpaths
5148
| test_free.cpp:85:12:85:12 | a | test_free.cpp:83:12:83:12 | pointer to operator delete output argument | test_free.cpp:85:12:85:12 | a | Memory pointed to by 'a' may already have been freed by $@. | test_free.cpp:83:5:83:13 | delete | delete |
5249
| test_free.cpp:103:10:103:10 | a | test_free.cpp:101:10:101:10 | pointer to free output argument | test_free.cpp:103:10:103:10 | a | Memory pointed to by 'a' may already have been freed by $@. | test_free.cpp:101:5:101:8 | call to free | call to free |
5350
| test_free.cpp:129:10:129:11 | * ... | test_free.cpp:128:10:128:11 | pointer to free output argument | test_free.cpp:129:10:129:11 | * ... | Memory pointed to by '* ...' may already have been freed by $@. | test_free.cpp:128:5:128:8 | call to free | call to free |
54-
| test_free.cpp:132:10:132:13 | access to array | test_free.cpp:131:10:131:13 | pointer to free output argument | test_free.cpp:132:10:132:13 | access to array | Memory pointed to by 'access to array' may already have been freed by $@. | test_free.cpp:131:5:131:8 | call to free | call to free |
5551
| test_free.cpp:154:10:154:10 | a | test_free.cpp:152:27:152:27 | pointer to free output argument | test_free.cpp:154:10:154:10 | a | Memory pointed to by 'a' may already have been freed by $@. | test_free.cpp:152:22:152:25 | call to free | call to free |
5652
| test_free.cpp:209:10:209:10 | a | test_free.cpp:207:10:207:10 | pointer to free output argument | test_free.cpp:209:10:209:10 | a | Memory pointed to by 'a' may already have been freed by $@. | test_free.cpp:207:5:207:8 | call to free | call to free |

‎cpp/ql/test/query-tests/Critical/MemoryFreed/test_free.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@ void test_ptr_deref(void ** a) {
129129
free(*a); // BAD
130130
*a = malloc(10);
131131
free(a[0]); // GOOD
132-
free(a[1]); // GOOD [FALSE POSITIVE]
132+
free(a[1]); // GOOD
133133
}
134134

135135
struct list {

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

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -108,9 +108,9 @@ private class MissingPinningSink extends DataFlow::Node {
108108
/** Configuration for finding uses of non trusted URLs. */
109109
private module UntrustedUrlConfig implements DataFlow::ConfigSig {
110110
predicate isSource(DataFlow::Node node) {
111-
trustedDomain(_) and
112111
exists(string lit | lit = node.asExpr().(CompileTimeConstantExpr).getStringValue() |
113112
lit.matches("%://%") and // it's a URL
113+
not lit.regexpMatch("^(classpath|file|jar):.*") and // discard non-network URIs
114114
not exists(string dom | trustedDomain(dom) and lit.matches("%" + dom + "%"))
115115
)
116116
}
@@ -121,16 +121,10 @@ private module UntrustedUrlConfig implements DataFlow::ConfigSig {
121121
private module UntrustedUrlFlow = TaintTracking::Global<UntrustedUrlConfig>;
122122

123123
/** Holds if `node` is a network communication call for which certificate pinning is not implemented. */
124-
predicate missingPinning(DataFlow::Node node, string domain) {
124+
predicate missingPinning(MissingPinningSink node, string domain) {
125125
isAndroid() and
126-
node instanceof MissingPinningSink and
127-
(
128-
not trustedDomain(_) and domain = ""
129-
or
130-
exists(DataFlow::Node src |
131-
UntrustedUrlFlow::flow(src, node) and
132-
domain = getDomain(src.asExpr())
133-
)
126+
exists(DataFlow::Node src | UntrustedUrlFlow::flow(src, node) |
127+
if trustedDomain(_) then domain = getDomain(src.asExpr()) else domain = ""
134128
)
135129
}
136130

‎java/ql/src/change-notes/2023-12-12-android-certificate-pinning-precision.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
* The query `java/android/missing-certificate-pinning` should no longer alert about requests pointing to the local filesystem.

‎java/ql/test/query-tests/security/CWE-295/AndroidMissingCertificatePinning/Test1/Test.java

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,24 @@
11
import java.net.URL;
22
import java.net.URLConnection;
33

4-
class Test{
4+
class Test {
55
URLConnection test1() throws Exception {
66
return new URL("https://good.example.com").openConnection();
77
}
88

99
URLConnection test2() throws Exception {
1010
return new URL("https://bad.example.com").openConnection(); // $hasUntrustedResult
1111
}
12-
}
12+
13+
URLConnection test3() throws Exception {
14+
return new URL("classpath:example/directory/test.class").openConnection();
15+
}
16+
17+
URLConnection test4() throws Exception {
18+
return new URL("file:///example/file").openConnection();
19+
}
20+
21+
URLConnection test5() throws Exception {
22+
return new URL("jar:file:///C:/example/test.jar!/test.xml").openConnection();
23+
}
24+
}

‎java/ql/test/query-tests/security/CWE-295/AndroidMissingCertificatePinning/Test2/Test.java

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,20 @@
11
import java.net.URL;
22
import java.net.URLConnection;
33

4-
class Test{
4+
class Test {
55
URLConnection test2() throws Exception {
66
return new URL("https://example.com").openConnection(); // $hasNoTrustedResult
77
}
8-
}
8+
9+
URLConnection test3() throws Exception {
10+
return new URL("classpath:example/directory/test.class").openConnection();
11+
}
12+
13+
URLConnection test4() throws Exception {
14+
return new URL("file:///example/file").openConnection();
15+
}
16+
17+
URLConnection test5() throws Exception {
18+
return new URL("jar:file:///C:/example/test.jar!/test.xml").openConnection();
19+
}
20+
}

‎java/ql/test/query-tests/security/CWE-295/AndroidMissingCertificatePinning/Test3/Test.java

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2,16 +2,21 @@
22
import okhttp3.CertificatePinner;
33
import okhttp3.Request;
44

5-
class Test{
5+
class Test {
66
void test1() throws Exception {
7-
CertificatePinner certificatePinner = new CertificatePinner.Builder()
8-
.add("good.example.com", "sha256/AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=")
9-
.build();
10-
OkHttpClient client = new OkHttpClient.Builder()
11-
.certificatePinner(certificatePinner)
12-
.build();
7+
CertificatePinner certificatePinner = new CertificatePinner.Builder()
8+
.add("good.example.com", "sha256/AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=")
9+
.build();
10+
OkHttpClient client =
11+
new OkHttpClient.Builder().certificatePinner(certificatePinner).build();
1312

14-
client.newCall(new Request.Builder().url("https://good.example.com").build()).execute();
15-
client.newCall(new Request.Builder().url("https://bad.example.com").build()).execute(); // $hasUntrustedResult
13+
client.newCall(new Request.Builder().url("https://good.example.com").build()).execute();
14+
client.newCall(new Request.Builder().url("https://bad.example.com").build()).execute(); // $hasUntrustedResult
15+
client.newCall(new Request.Builder().url("classpath:example/directory/test.class").build())
16+
.execute();
17+
client.newCall(new Request.Builder().url("file:///example/file").build()).execute();
18+
client.newCall(
19+
new Request.Builder().url("jar:file:///C:/example/test.jar!/test.xml").build())
20+
.execute();
1621
}
17-
}
22+
}

‎java/ql/test/query-tests/security/CWE-295/AndroidMissingCertificatePinning/Test4/Test.java

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,19 +8,20 @@
88
import javax.net.ssl.SSLContext;
99
import android.content.res.Resources;
1010

11-
class Test{
11+
class Test {
1212
void test1(Resources resources) throws Exception {
1313
KeyStore keyStore = KeyStore.getInstance("BKS");
1414
keyStore.load(resources.openRawResource(R.raw.cert), null);
1515

16-
TrustManagerFactory tmf = TrustManagerFactory.getInstance(TrustManagerFactory.getDefaultAlgorithm());
16+
TrustManagerFactory tmf =
17+
TrustManagerFactory.getInstance(TrustManagerFactory.getDefaultAlgorithm());
1718
tmf.init(keyStore);
1819

1920
SSLContext sslContext = SSLContext.getInstance("TLS");
2021
sslContext.init(null, tmf.getTrustManagers(), null);
2122

2223
URL url = new URL("http://www.example.com/");
23-
HttpsURLConnection urlConnection = (HttpsURLConnection) url.openConnection();
24+
HttpsURLConnection urlConnection = (HttpsURLConnection) url.openConnection();
2425

2526
urlConnection.setSSLSocketFactory(sslContext.getSocketFactory());
2627
}
@@ -29,4 +30,4 @@ void test2() throws Exception {
2930
URL url = new URL("http://www.example.com/");
3031
HttpsURLConnection urlConnection = (HttpsURLConnection) url.openConnection(); // $hasNoTrustedResult
3132
}
32-
}
33+
}

0 commit comments

Comments
 (0)