Skip to content

Commit 5a0f3af

Browse files
committed
Add characterFilteringCallStep to java/sensitive-log
1 parent 077084c commit 5a0f3af

4 files changed

Lines changed: 212 additions & 9 deletions

File tree

Lines changed: 161 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,161 @@
1+
/**
2+
* Provides support for identifying callables that copy characters from an input string while
3+
* possibly dropping or replacing some characters.
4+
*/
5+
6+
import java
7+
import semmle.code.java.dataflow.DataFlow
8+
9+
private predicate isStringLike(Type type) {
10+
type.(RefType).getASupertype*().hasQualifiedName("java.lang", "CharSequence")
11+
}
12+
13+
private predicate isReader(Type type) {
14+
type.(RefType).getASupertype*().hasQualifiedName("java.io", "Reader")
15+
}
16+
17+
private predicate isCharacterOutput(Type type) {
18+
type.(Array).getElementType().(PrimitiveType).hasName("char")
19+
or
20+
type.(RefType).getASupertype*().hasQualifiedName("java.lang", "Appendable")
21+
or
22+
type.(RefType).getASupertype*().hasQualifiedName("java.io", "Writer")
23+
}
24+
25+
private predicate occursInLoop(Expr expr, LoopStmt loop) {
26+
expr.getEnclosingStmt().getEnclosingStmt*() = loop
27+
}
28+
29+
/**
30+
* Holds if `read` is a character or code point read from `input` during `loop`.
31+
*/
32+
private predicate readsCharacter(Parameter input, LoopStmt loop, Expr read) {
33+
exists(MethodCall call, VarAccess inputAccess |
34+
inputAccess.getVariable() = input and
35+
DataFlow::localExprFlow(inputAccess, call.getQualifier()) and
36+
call.getMethod().hasName("charAt") and
37+
read = call and
38+
occursInLoop(call, loop)
39+
)
40+
or
41+
exists(MethodCall call, VarAccess inputAccess |
42+
inputAccess.getVariable() = input and
43+
DataFlow::localExprFlow(inputAccess, call.getArgument(0)) and
44+
call.getMethod().hasQualifiedName("java.lang", "Character", "codePointAt") and
45+
read = call and
46+
occursInLoop(call, loop)
47+
)
48+
or
49+
exists(EnhancedForStmt forStmt, MethodCall toCharArray, VarAccess inputAccess |
50+
loop = forStmt and
51+
forStmt.getExpr() = toCharArray and
52+
toCharArray.getMethod().hasName("toCharArray") and
53+
inputAccess.getVariable() = input and
54+
DataFlow::localExprFlow(inputAccess, toCharArray.getQualifier()) and
55+
read = forStmt.getVariable().getVariable().getAnAccess() and
56+
occursInLoop(read, loop)
57+
)
58+
or
59+
exists(
60+
LocalVariableDecl reader, ClassInstanceExpr readerInit, Expr constructorArgument,
61+
VarAccess inputAccess, MethodCall readCall
62+
|
63+
reader.getType() = readerInit.getType() and
64+
isReader(reader.getType()) and
65+
reader.getInitializer() = readerInit and
66+
constructorArgument = readerInit.getAnArgument() and
67+
inputAccess.getVariable() = input and
68+
DataFlow::localExprFlow(inputAccess, constructorArgument) and
69+
readCall.getQualifier() = reader.getAnAccess() and
70+
readCall.getMethod().hasName("read") and
71+
read = readCall and
72+
occursInLoop(readCall, loop)
73+
)
74+
}
75+
76+
/**
77+
* Holds if a character value can flow from `source` to `sink`.
78+
*
79+
* This is deliberately separate from normal taint tracking: character conversion is used only as
80+
* evidence for inferring a string-level summary.
81+
*/
82+
private predicate characterFlowStep(DataFlow::Node source, DataFlow::Node sink) {
83+
DataFlow::localFlowStep(source, sink)
84+
or
85+
exists(MethodCall conversion |
86+
conversion.getMethod().hasQualifiedName("java.lang", "Character", ["toChars", "toString"]) and
87+
source.asExpr() = conversion.getArgument(0) and
88+
sink.asExpr() = conversion
89+
)
90+
}
91+
92+
private predicate characterFlows(Expr source, Expr sink) {
93+
characterFlowStep*(DataFlow::exprNode(source), DataFlow::exprNode(sink))
94+
}
95+
96+
/**
97+
* Holds if `output` receives a character derived from `read` during `loop`.
98+
*/
99+
private predicate writesCharacter(LoopStmt loop, Expr read, Variable output) {
100+
exists(MethodCall write, Expr value |
101+
occursInLoop(write, loop) and
102+
write.getMethod().hasName(["append", "write"]) and
103+
write.getQualifier() = output.getAnAccess() and
104+
isCharacterOutput(output.getType()) and
105+
value = write.getArgument(0) and
106+
characterFlows(read, value)
107+
)
108+
or
109+
exists(AssignExpr write, ArrayAccess element |
110+
occursInLoop(write, loop) and
111+
write.getDest() = element and
112+
element.getArray() = output.getAnAccess() and
113+
isCharacterOutput(output.getType()) and
114+
characterFlows(read, write.getSource())
115+
)
116+
or
117+
exists(AssignAddExpr append |
118+
occursInLoop(append, loop) and
119+
append.getDest() = output.getAnAccess() and
120+
output.getType() instanceof TypeString and
121+
characterFlows(read, append.getSource())
122+
)
123+
}
124+
125+
private predicate returnsOutput(Method method, Variable output) {
126+
exists(ReturnStmt returnStmt |
127+
returnStmt.getEnclosingCallable() = method and
128+
returnStmt.getExpr().getAChildExpr*() = output.getAnAccess()
129+
)
130+
}
131+
132+
private predicate characterFilteringOutput(
133+
Method method, int inputPosition, LocalScopeVariable output
134+
) {
135+
exists(Parameter input, LoopStmt loop, Expr read |
136+
input = method.getParameter(inputPosition) and
137+
isStringLike(input.getType()) and
138+
loop.getEnclosingCallable() = method and
139+
output.getCallable() = method
140+
|
141+
readsCharacter(input, loop, read) and
142+
writesCharacter(loop, read, output)
143+
)
144+
}
145+
146+
/**
147+
* Holds if a call to a character-filtering method copies data from `input` to `output`.
148+
*/
149+
predicate characterFilteringCallStep(DataFlow::Node input, DataFlow::Node output) {
150+
exists(MethodCall call, int inputPosition, LocalScopeVariable filteredOutput |
151+
characterFilteringOutput(call.getMethod(), inputPosition, filteredOutput) and
152+
input.asExpr() = call.getArgument(inputPosition)
153+
|
154+
returnsOutput(call.getMethod(), filteredOutput) and output.asExpr() = call
155+
or
156+
exists(int outputPosition |
157+
filteredOutput = call.getMethod().getParameter(outputPosition) and
158+
output.asExpr() = call.getArgument(outputPosition)
159+
)
160+
)
161+
}

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import java
44
private import semmle.code.java.dataflow.ExternalFlow
55
import semmle.code.java.dataflow.TaintTracking
6+
import semmle.code.java.dataflow.CharacterFiltering
67
import semmle.code.java.security.SensitiveActions
78
import semmle.code.java.frameworks.android.Compose
89
private import semmle.code.java.security.Sanitizers
@@ -138,6 +139,10 @@ module SensitiveLoggerConfig implements DataFlow::ConfigSig {
138139

139140
predicate isBarrierIn(DataFlow::Node node) { isSource(node) }
140141

142+
predicate isAdditionalFlowStep(DataFlow::Node source, DataFlow::Node sink) {
143+
characterFilteringCallStep(source, sink)
144+
}
145+
141146
predicate observeDiffInformedIncrementalMode() { any() }
142147
}
143148

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

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ class CharacterFilteringTest {
88
private final Logger logger = null;
99

1010
void testStringBuilder(String password) {
11-
logger.info(removeNewlines(password)); // $ MISSING: Alert
11+
logger.info(removeNewlines(password)); // $ Alert
1212
}
1313

1414
static String removeNewlines(String input) {
@@ -23,7 +23,7 @@ static String removeNewlines(String input) {
2323
}
2424

2525
void testReplacement(String authToken) {
26-
logger.info(escapeControlCharacters(authToken)); // $ MISSING: Alert
26+
logger.info(escapeControlCharacters(authToken)); // $ Alert
2727
}
2828

2929
static String escapeControlCharacters(String input) {
@@ -45,7 +45,7 @@ static String escapeControlCharacters(String input) {
4545
}
4646

4747
void testWriterParameter(String accessToken) throws IOException {
48-
logger.info(copyPrintableCodePoints(accessToken)); // $ MISSING: Alert
48+
logger.info(copyPrintableCodePoints(accessToken)); // $ Alert
4949
}
5050

5151
static String copyPrintableCodePoints(String input) throws IOException {
@@ -67,7 +67,7 @@ static void copyPrintableCodePoints(CharSequence input, Writer output) throws IO
6767
}
6868

6969
void testOutputArray(String clientSecret) {
70-
logger.info(copyAsciiCharacters(clientSecret)); // $ MISSING: Alert
70+
logger.info(copyAsciiCharacters(clientSecret)); // $ Alert
7171
}
7272

7373
static String copyAsciiCharacters(String input) {
@@ -83,7 +83,7 @@ static String copyAsciiCharacters(String input) {
8383
}
8484

8585
void testConcatenation(String apiSecret) {
86-
logger.info(removeWhitespace(apiSecret)); // $ MISSING: Alert
86+
logger.info(removeWhitespace(apiSecret)); // $ Alert
8787
}
8888

8989
static String removeWhitespace(String input) {
@@ -98,7 +98,7 @@ static String removeWhitespace(String input) {
9898
}
9999

100100
void testReaderWriter(String sessionToken) throws IOException {
101-
logger.info(copyWithoutLineBreaks(sessionToken)); // $ MISSING: Alert
101+
logger.info(copyWithoutLineBreaks(sessionToken)); // $ Alert
102102
}
103103

104104
static String copyWithoutLineBreaks(String input) throws IOException {

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

Lines changed: 40 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,10 @@
11
#select
2+
| CharacterFilteringTest.java:11:21:11:44 | removeNewlines(...) | CharacterFilteringTest.java:11:36:11:43 | password : String | CharacterFilteringTest.java:11:21:11:44 | removeNewlines(...) | This $@ is written to a log file. | CharacterFilteringTest.java:11:36:11:43 | password | potentially sensitive information |
3+
| CharacterFilteringTest.java:26:21:26:54 | escapeControlCharacters(...) | CharacterFilteringTest.java:26:45:26:53 | authToken : String | CharacterFilteringTest.java:26:21:26:54 | escapeControlCharacters(...) | This $@ is written to a log file. | CharacterFilteringTest.java:26:45:26:53 | authToken | potentially sensitive information |
4+
| CharacterFilteringTest.java:48:21:48:56 | copyPrintableCodePoints(...) | CharacterFilteringTest.java:48:45:48:55 | accessToken : String | CharacterFilteringTest.java:48:21:48:56 | copyPrintableCodePoints(...) | This $@ is written to a log file. | CharacterFilteringTest.java:48:45:48:55 | accessToken | potentially sensitive information |
5+
| CharacterFilteringTest.java:70:21:70:53 | copyAsciiCharacters(...) | CharacterFilteringTest.java:70:41:70:52 | clientSecret : String | CharacterFilteringTest.java:70:21:70:53 | copyAsciiCharacters(...) | This $@ is written to a log file. | CharacterFilteringTest.java:70:41:70:52 | clientSecret | potentially sensitive information |
6+
| CharacterFilteringTest.java:86:21:86:47 | removeWhitespace(...) | CharacterFilteringTest.java:86:38:86:46 | apiSecret : String | CharacterFilteringTest.java:86:21:86:47 | removeWhitespace(...) | This $@ is written to a log file. | CharacterFilteringTest.java:86:38:86:46 | apiSecret | potentially sensitive information |
7+
| CharacterFilteringTest.java:101:21:101:55 | copyWithoutLineBreaks(...) | CharacterFilteringTest.java:101:43:101:54 | sessionToken : String | CharacterFilteringTest.java:101:21:101:55 | copyWithoutLineBreaks(...) | This $@ is written to a log file. | CharacterFilteringTest.java:101:43:101:54 | sessionToken | potentially sensitive information |
28
| 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 |
39
| 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 |
410
| 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 |
@@ -13,11 +19,23 @@
1319
| Test.java:81:21:81:44 | ... + ... | Test.java:81:33:81:44 | refreshToken : String | Test.java:81:21:81:44 | ... + ... | This $@ is written to a log file. | Test.java:81:33:81:44 | refreshToken | potentially sensitive information |
1420
| Test.java:82:21:82:43 | ... + ... | Test.java:82:33:82:43 | secretValue : String | Test.java:82:21:82:43 | ... + ... | This $@ is written to a log file. | Test.java:82:33:82:43 | secretValue | potentially sensitive information |
1521
edges
22+
| CharacterFilteringTest.java:11:36:11:43 | password : String | CharacterFilteringTest.java:11:21:11:44 | removeNewlines(...) | provenance | Config Sink:MaD:2 |
23+
| CharacterFilteringTest.java:26:45:26:53 | authToken : String | CharacterFilteringTest.java:26:21:26:54 | escapeControlCharacters(...) | provenance | Config Sink:MaD:2 |
24+
| CharacterFilteringTest.java:48:45:48:55 | accessToken : String | CharacterFilteringTest.java:48:21:48:56 | copyPrintableCodePoints(...) | provenance | Config Sink:MaD:2 |
25+
| CharacterFilteringTest.java:48:45:48:55 | accessToken : String | CharacterFilteringTest.java:51:43:51:54 | input : String | provenance | |
26+
| CharacterFilteringTest.java:51:43:51:54 | input : String | CharacterFilteringTest.java:53:33:53:37 | input : String | provenance | |
27+
| CharacterFilteringTest.java:53:33:53:37 | input : String | CharacterFilteringTest.java:53:40:53:45 | output : StringWriter | provenance | Config |
28+
| CharacterFilteringTest.java:53:40:53:45 | output : StringWriter | CharacterFilteringTest.java:54:16:54:21 | output : StringWriter | provenance | |
29+
| CharacterFilteringTest.java:53:40:53:45 | output : StringWriter | CharacterFilteringTest.java:54:16:54:32 | toString(...) : String | provenance | %StringWriter |
30+
| CharacterFilteringTest.java:54:16:54:21 | output : StringWriter | CharacterFilteringTest.java:54:16:54:32 | toString(...) : String | provenance | MaD:3 |
31+
| CharacterFilteringTest.java:70:41:70:52 | clientSecret : String | CharacterFilteringTest.java:70:21:70:53 | copyAsciiCharacters(...) | provenance | Config Sink:MaD:2 |
32+
| CharacterFilteringTest.java:86:38:86:46 | apiSecret : String | CharacterFilteringTest.java:86:21:86:47 | removeWhitespace(...) | provenance | Config Sink:MaD:2 |
33+
| CharacterFilteringTest.java:101:43:101:54 | sessionToken : String | CharacterFilteringTest.java:101:21:101:55 | copyWithoutLineBreaks(...) | provenance | Config Sink:MaD:2 |
1634
| Test.java:11:46:11:53 | password : String | Test.java:11:21:11:53 | ... + ... | provenance | Sink:MaD:2 |
1735
| Test.java:12:44:12:52 | authToken : String | Test.java:12:22:12:52 | ... + ... | provenance | Sink:MaD:1 |
18-
| Test.java:21:44:21:52 | authToken : String | Test.java:21:44:21:67 | substring(...) : String | provenance | MaD:3 |
36+
| Test.java:21:44:21:52 | authToken : String | Test.java:21:44:21:67 | substring(...) : String | provenance | MaD:4 |
1937
| Test.java:21:44:21:67 | substring(...) : String | Test.java:21:22:21:75 | ... + ... | provenance | Sink:MaD:1 |
20-
| Test.java:22:44:22:52 | authToken : String | Test.java:22:44:22:67 | substring(...) : String | provenance | MaD:3 |
38+
| Test.java:22:44:22:52 | authToken : String | Test.java:22:44:22:67 | substring(...) : String | provenance | MaD:4 |
2139
| Test.java:22:44:22:67 | substring(...) : String | Test.java:22:22:22:75 | ... + ... | provenance | Sink:MaD:1 |
2240
| Test.java:31:30:31:37 | password : String | Test.java:31:21:31:37 | ... + ... | provenance | Sink:MaD:2 |
2341
| Test.java:75:33:75:43 | accessToken : String | Test.java:75:21:75:43 | ... + ... | provenance | Sink:MaD:2 |
@@ -31,8 +49,26 @@ edges
3149
models
3250
| 1 | Sink: org.apache.logging.log4j; Logger; true; error; (String); ; Argument[0]; log-injection; manual |
3351
| 2 | Sink: org.apache.logging.log4j; Logger; true; info; (String); ; Argument[0]; log-injection; manual |
34-
| 3 | Summary: java.lang; String; false; substring; ; ; Argument[this]; ReturnValue; taint; manual |
52+
| 3 | Summary: java.io; StringWriter; false; toString; (); ; Argument[this]; ReturnValue; taint; manual |
53+
| 4 | Summary: java.lang; String; false; substring; ; ; Argument[this]; ReturnValue; taint; manual |
3554
nodes
55+
| CharacterFilteringTest.java:11:21:11:44 | removeNewlines(...) | semmle.label | removeNewlines(...) |
56+
| CharacterFilteringTest.java:11:36:11:43 | password : String | semmle.label | password : String |
57+
| CharacterFilteringTest.java:26:21:26:54 | escapeControlCharacters(...) | semmle.label | escapeControlCharacters(...) |
58+
| CharacterFilteringTest.java:26:45:26:53 | authToken : String | semmle.label | authToken : String |
59+
| CharacterFilteringTest.java:48:21:48:56 | copyPrintableCodePoints(...) | semmle.label | copyPrintableCodePoints(...) |
60+
| CharacterFilteringTest.java:48:45:48:55 | accessToken : String | semmle.label | accessToken : String |
61+
| CharacterFilteringTest.java:51:43:51:54 | input : String | semmle.label | input : String |
62+
| CharacterFilteringTest.java:53:33:53:37 | input : String | semmle.label | input : String |
63+
| CharacterFilteringTest.java:53:40:53:45 | output : StringWriter | semmle.label | output : StringWriter |
64+
| CharacterFilteringTest.java:54:16:54:21 | output : StringWriter | semmle.label | output : StringWriter |
65+
| CharacterFilteringTest.java:54:16:54:32 | toString(...) : String | semmle.label | toString(...) : String |
66+
| CharacterFilteringTest.java:70:21:70:53 | copyAsciiCharacters(...) | semmle.label | copyAsciiCharacters(...) |
67+
| CharacterFilteringTest.java:70:41:70:52 | clientSecret : String | semmle.label | clientSecret : String |
68+
| CharacterFilteringTest.java:86:21:86:47 | removeWhitespace(...) | semmle.label | removeWhitespace(...) |
69+
| CharacterFilteringTest.java:86:38:86:46 | apiSecret : String | semmle.label | apiSecret : String |
70+
| CharacterFilteringTest.java:101:21:101:55 | copyWithoutLineBreaks(...) | semmle.label | copyWithoutLineBreaks(...) |
71+
| CharacterFilteringTest.java:101:43:101:54 | sessionToken : String | semmle.label | sessionToken : String |
3672
| Test.java:11:21:11:53 | ... + ... | semmle.label | ... + ... |
3773
| Test.java:11:46:11:53 | password : String | semmle.label | password : String |
3874
| Test.java:12:22:12:52 | ... + ... | semmle.label | ... + ... |
@@ -62,3 +98,4 @@ nodes
6298
| Test.java:82:21:82:43 | ... + ... | semmle.label | ... + ... |
6399
| Test.java:82:33:82:43 | secretValue : String | semmle.label | secretValue : String |
64100
subpaths
101+
| CharacterFilteringTest.java:48:45:48:55 | accessToken : String | CharacterFilteringTest.java:51:43:51:54 | input : String | CharacterFilteringTest.java:54:16:54:32 | toString(...) : String | CharacterFilteringTest.java:48:21:48:56 | copyPrintableCodePoints(...) |

0 commit comments

Comments
 (0)