Skip to content

Commit ec381e4

Browse files
owen-mcowen-mc
authored
Use range analysis and improve tests
1 parent ce13668 commit ec381e4

File tree

3 files changed

+42
-33
lines changed

3 files changed

+42
-33
lines changed

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

Lines changed: 15 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -87,26 +87,28 @@ private class PrefixSuffixBarrier extends SensitiveLoggerBarrier {
8787
bindingset[limit, isKotlin]
8888
private predicate singleArgLimit(MethodCall mc, int limit, boolean isKotlin) {
8989
mc.getNumArgument() = 1 and
90-
exists(int firstArgIndex |
91-
(if isKotlin = true then firstArgIndex = 1 else firstArgIndex = 0) and
92-
mc.getArgument(firstArgIndex).getUnderlyingExpr().(CompileTimeConstantExpr).getIntValue() <=
93-
limit
90+
exists(int firstArgIndex, int delta |
91+
if isKotlin = true then firstArgIndex = 1 else firstArgIndex = 0
92+
|
93+
bounded(mc.getArgument(firstArgIndex).getUnderlyingExpr(), any(ZeroBound z), delta, true, _) and
94+
delta <= limit
9495
)
9596
}
9697

9798
/** A predicate to check two-argument method calls for zero and a constant integer below a set limit. */
9899
bindingset[limit, isKotlin]
99100
private predicate twoArgLimit(MethodCall mc, int limit, boolean isKotlin) {
100101
mc.getNumArgument() = 2 and
101-
exists(int firstArgIndex, int secondArgIndex |
102-
(
103-
isKotlin = true and firstArgIndex = 1 and secondArgIndex = 2
104-
or
105-
isKotlin = false and firstArgIndex = 0 and secondArgIndex = 1
106-
) and
107-
mc.getArgument(firstArgIndex).getUnderlyingExpr().(CompileTimeConstantExpr).getIntValue() = 0 and
108-
mc.getArgument(secondArgIndex).getUnderlyingExpr().(CompileTimeConstantExpr).getIntValue() <=
109-
limit
102+
exists(int firstArgIndex, int secondArgIndex, int delta |
103+
isKotlin = true and firstArgIndex = 1 and secondArgIndex = 2
104+
or
105+
isKotlin = false and firstArgIndex = 0 and secondArgIndex = 1
106+
|
107+
// mc.getArgument(firstArgIndex).getUnderlyingExpr().(CompileTimeConstantExpr).getIntValue() = 0 and
108+
bounded(mc.getArgument(firstArgIndex).getUnderlyingExpr(), any(ZeroBound z), 0, true, _) and
109+
bounded(mc.getArgument(firstArgIndex).getUnderlyingExpr(), any(ZeroBound z), 0, false, _) and
110+
bounded(mc.getArgument(secondArgIndex).getUnderlyingExpr(), any(ZeroBound z), delta, true, _) and
111+
delta <= limit
110112
)
111113
}
112114

‎java/ql/test/query-tests/security/CWE-532/SensitiveLogInfo.expected‎

Lines changed: 20 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,28 +1,28 @@
11
#select
2-
| Test.java:7:21:7:53 | ... + ... | Test.java:7:46:7:53 | password : String | Test.java:7:21:7:53 | ... + ... | This $@ is written to a log file. | Test.java:7:46:7:53 | password | potentially sensitive information |
3-
| Test.java:8:22:8:52 | ... + ... | Test.java:8:44:8:52 | authToken : String | Test.java:8:22:8:52 | ... + ... | This $@ is written to a log file. | Test.java:8:44:8:52 | authToken | potentially sensitive information |
4-
| Test.java:14:22:14:75 | ... + ... | Test.java:14:44:14:52 | authToken : String | Test.java:14:22:14:75 | ... + ... | This $@ is written to a log file. | Test.java:14:44:14:52 | authToken | potentially sensitive information |
5-
| Test.java:15:22:15:75 | ... + ... | Test.java:15:44:15:52 | authToken : String | Test.java:15:22:15:75 | ... + ... | This $@ is written to a log file. | Test.java:15:44:15:52 | authToken | potentially sensitive information |
2+
| Test.java:11:21:11:53 | ... + ... | Test.java:11:46:11:53 | password : String | Test.java:11:21:11:53 | ... + ... | This $@ is written to a log file. | Test.java:11:46:11:53 | password | potentially sensitive information |
3+
| Test.java:12:22:12:52 | ... + ... | Test.java:12:44:12:52 | authToken : String | Test.java:12:22:12:52 | ... + ... | This $@ is written to a log file. | Test.java:12:44:12:52 | authToken | potentially sensitive information |
4+
| Test.java:21:22:21:75 | ... + ... | Test.java:21:44:21:52 | authToken : String | Test.java:21:22:21:75 | ... + ... | This $@ is written to a log file. | Test.java:21:44:21:52 | authToken | potentially sensitive information |
5+
| Test.java:22:22:22:75 | ... + ... | Test.java:22:44:22:52 | authToken : String | Test.java:22:22:22:75 | ... + ... | This $@ is written to a log file. | Test.java:22:44:22:52 | authToken | potentially sensitive information |
66
edges
7-
| Test.java:7:46:7:53 | password : String | Test.java:7:21:7:53 | ... + ... | provenance | Sink:MaD:2 |
8-
| Test.java:8:44:8:52 | authToken : String | Test.java:8:22:8:52 | ... + ... | provenance | Sink:MaD:1 |
9-
| Test.java:14:44:14:52 | authToken : String | Test.java:14:44:14:67 | substring(...) : String | provenance | MaD:3 |
10-
| Test.java:14:44:14:67 | substring(...) : String | Test.java:14:22:14:75 | ... + ... | provenance | Sink:MaD:1 |
11-
| Test.java:15:44:15:52 | authToken : String | Test.java:15:44:15:67 | substring(...) : String | provenance | MaD:3 |
12-
| Test.java:15:44:15:67 | substring(...) : String | Test.java:15:22:15:75 | ... + ... | provenance | Sink:MaD:1 |
7+
| Test.java:11:46:11:53 | password : String | Test.java:11:21:11:53 | ... + ... | provenance | Sink:MaD:2 |
8+
| Test.java:12:44:12:52 | authToken : String | Test.java:12:22:12:52 | ... + ... | provenance | Sink:MaD:1 |
9+
| Test.java:21:44:21:52 | authToken : String | Test.java:21:44:21:67 | substring(...) : String | provenance | MaD:3 |
10+
| Test.java:21:44:21:67 | substring(...) : String | Test.java:21:22:21:75 | ... + ... | provenance | Sink:MaD:1 |
11+
| Test.java:22:44:22:52 | authToken : String | Test.java:22:44:22:67 | substring(...) : String | provenance | MaD:3 |
12+
| Test.java:22:44:22:67 | substring(...) : String | Test.java:22:22:22:75 | ... + ... | provenance | Sink:MaD:1 |
1313
models
1414
| 1 | Sink: org.apache.logging.log4j; Logger; true; error; (String); ; Argument[0]; log-injection; manual |
1515
| 2 | Sink: org.apache.logging.log4j; Logger; true; info; (String); ; Argument[0]; log-injection; manual |
1616
| 3 | Summary: java.lang; String; false; substring; ; ; Argument[this]; ReturnValue; taint; manual |
1717
nodes
18-
| Test.java:7:21:7:53 | ... + ... | semmle.label | ... + ... |
19-
| Test.java:7:46:7:53 | password : String | semmle.label | password : String |
20-
| Test.java:8:22:8:52 | ... + ... | semmle.label | ... + ... |
21-
| Test.java:8:44:8:52 | authToken : String | semmle.label | authToken : String |
22-
| Test.java:14:22:14:75 | ... + ... | semmle.label | ... + ... |
23-
| Test.java:14:44:14:52 | authToken : String | semmle.label | authToken : String |
24-
| Test.java:14:44:14:67 | substring(...) : String | semmle.label | substring(...) : String |
25-
| Test.java:15:22:15:75 | ... + ... | semmle.label | ... + ... |
26-
| Test.java:15:44:15:52 | authToken : String | semmle.label | authToken : String |
27-
| Test.java:15:44:15:67 | substring(...) : String | semmle.label | substring(...) : String |
18+
| Test.java:11:21:11:53 | ... + ... | semmle.label | ... + ... |
19+
| Test.java:11:46:11:53 | password : String | semmle.label | password : String |
20+
| Test.java:12:22:12:52 | ... + ... | semmle.label | ... + ... |
21+
| Test.java:12:44:12:52 | authToken : String | semmle.label | authToken : String |
22+
| Test.java:21:22:21:75 | ... + ... | semmle.label | ... + ... |
23+
| Test.java:21:44:21:52 | authToken : String | semmle.label | authToken : String |
24+
| Test.java:21:44:21:67 | substring(...) : String | semmle.label | substring(...) : String |
25+
| Test.java:22:22:22:75 | ... + ... | semmle.label | ... + ... |
26+
| Test.java:22:44:22:52 | authToken : String | semmle.label | authToken : String |
27+
| Test.java:22:44:22:67 | substring(...) : String | semmle.label | substring(...) : String |
2828
subpaths

‎java/ql/test/query-tests/security/CWE-532/Test.java‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,14 +3,21 @@
33
class Test {
44
void test(String password, String authToken, String username, String nullToken, String stringTokenizer) {
55
Logger logger = null;
6+
int zero = 0;
7+
int four = 4;
8+
short zeroS = 0;
9+
long fourL = 4;
610

711
logger.info("User's password is: " + password); // $ Alert
812
logger.error("Auth failed for: " + authToken); // $ Alert
913
logger.error("Auth failed for: " + username); // Safe
1014
logger.error("Auth failed for: " + nullToken); // Safe
1115
logger.error("Auth failed for: " + stringTokenizer); // Safe
1216
logger.error("Auth failed for: " + authToken.substring(4) + "..."); // Safe
17+
logger.error("Auth failed for: " + authToken.substring(four) + "..."); // Safe
1318
logger.error("Auth failed for: " + authToken.substring(0,4) + "..."); // Safe
19+
logger.error("Auth failed for: " + authToken.substring(zero,four) + "..."); // Safe
20+
logger.error("Auth failed for: " + authToken.substring((int)zeroS,(int)fourL) + "..."); // Safe
1421
logger.error("Auth failed for: " + authToken.substring(1,5) + "..."); // $ Alert
1522
logger.error("Auth failed for: " + authToken.substring(0,8) + "..."); // $ Alert
1623
}

0 commit comments

Comments
 (0)