Skip to content

Commit 36cb207

Browse files
joefarebrotherjoefarebrother
committed
Increase precision of tests to test value flow
1 parent 678597f commit 36cb207

File tree

4 files changed

+53
-27
lines changed

4 files changed

+53
-27
lines changed

‎java/ql/src/semmle/code/java/frameworks/guava/Base.qll

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ private class GuavaBaseCsv extends SummaryModelCsv {
5151
"com.google.common.base;Splitter;false;splitToStream;(CharSequence);;Argument[0];ReturnValue;taint",
5252
"com.google.common.base;Splitter$MapSplitter;false;split;(CharSequence);;Argument[0];ReturnValue;taint",
5353
"com.google.common.base;Preconditions;false;checkNotNull;;;Argument[0];ReturnValue;value",
54-
"com.google.common.base;Verify;false;verifyNotNull;;;Argument[0];ReturnValue;taint",
54+
"com.google.common.base;Verify;false;verifyNotNull;;;Argument[0];ReturnValue;value",
5555
"com.google.common.base;Ascii;false;toLowerCase;(CharSequence);;Argument[0];ReturnValue;taint",
5656
"com.google.common.base;Ascii;false;toLowerCase;(String);;Argument[0];ReturnValue;taint",
5757
"com.google.common.base;Ascii;false;toUpperCase;(CharSequence);;Argument[0];ReturnValue;taint",
@@ -72,9 +72,10 @@ private class GuavaBaseCsv extends SummaryModelCsv {
7272
"com.google.common.base;Optional;true;get;();;Element of Argument[-1];ReturnValue;value",
7373
"com.google.common.base;Optional;true;asSet;();;Element of Argument[-1];Element of ReturnValue;value",
7474
"com.google.common.base;Optional;true;of;(Object);;Argument[0];Element of ReturnValue;value",
75-
"com.google.common.base;Optional;true;or;;;Element of Argument[-1];ReturnValue;value",
76-
"com.google.common.base;Optional;true;or;(Optional);;Element of Argument[0];ReturnValue;value",
77-
"com.google.common.base;Optional;true;or;(Supplier);;Argument[0];ReturnValue;value",
75+
"com.google.common.base;Optional;true;or;(Optional);;Element of Argument[-1..0];Element of ReturnValue;value",
76+
"com.google.common.base;Optional;true;or;(Supplier);;Element of Argument[-1];ReturnValue;value",
77+
"com.google.common.base;Optional;true;or;(Supplier);;Argument[0];ReturnValue;taint",
78+
"com.google.common.base;Optional;true;or;(Object);;Element of Argument[-1];ReturnValue;value",
7879
"com.google.common.base;Optional;true;or;(Object);;Argument[0];ReturnValue;value",
7980
"com.google.common.base;Optional;true;orNull;();;Element of Argument[-1];ReturnValue;value",
8081
"com.google.common.base;Optional;true;presentInstances;(Iterable);;Element of Element of Argument[0];Element of ReturnValue;value",

‎java/ql/test/library-tests/frameworks/guava/TestBase.java

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import java.util.Map;
44
import java.util.HashMap;
55
import java.util.concurrent.TimeUnit;
6+
import java.util.Set;
67

78
class TestBase {
89
String taint() { return "tainted"; }
@@ -15,7 +16,7 @@ void test1() {
1516
sink(Strings.padStart(x, 10, ' ')); // $numTaintFlow=1
1617
sink(Strings.padEnd(x, 10, ' ')); // $numTaintFlow=1
1718
sink(Strings.repeat(x, 3)); // $numTaintFlow=1
18-
sink(Strings.emptyToNull(Strings.nullToEmpty(x))); // $numTaintFlow=1
19+
sink(Strings.emptyToNull(Strings.nullToEmpty(x))); // $numValueFlow=1
1920
sink(Strings.lenientFormat(x, 3)); // $numTaintFlow=1
2021
sink(Strings.commonPrefix(x, "abc"));
2122
sink(Strings.commonSuffix(x, "cde"));
@@ -59,8 +60,8 @@ void test3() {
5960
}
6061

6162
void test4() {
62-
sink(Preconditions.checkNotNull(taint())); // $numTaintFlow=1
63-
sink(Verify.verifyNotNull(taint())); // $numTaintFlow=1
63+
sink(Preconditions.checkNotNull(taint())); // $numValueFlow=1
64+
sink(Verify.verifyNotNull(taint())); // $numValueFlow=1
6465
}
6566

6667
void test5() {
@@ -78,9 +79,9 @@ void test6() {
7879
}
7980

8081
void test7() {
81-
sink(MoreObjects.firstNonNull(taint(), taint())); // $numTaintFlow=2
82-
sink(MoreObjects.firstNonNull(null, taint())); // $numTaintFlow=1
83-
sink(MoreObjects.firstNonNull(taint(), null)); // $numTaintFlow=1
82+
sink(MoreObjects.firstNonNull(taint(), taint())); // $numValueFlow=2
83+
sink(MoreObjects.firstNonNull(null, taint())); // $numValueFlow=1
84+
sink(MoreObjects.firstNonNull(taint(), null)); // $numValueFlow=1
8485
sink(MoreObjects.toStringHelper(taint()).add("x", 3).omitNullValues().toString()); // $numTaintFlow=1
8586
sink(MoreObjects.toStringHelper((Object) taint()).toString());
8687
sink(MoreObjects.toStringHelper("a").add("x", 3).add(taint(), 4).toString()); // $numTaintFlow=1
@@ -94,16 +95,15 @@ void test7() {
9495
void test8() {
9596
Optional<String> x = Optional.of(taint());
9697
sink(x); // $numTaintFlow=1
97-
sink(x.get()); // $numTaintFlow=1
98-
sink(x.or("hi")); // $numTaintFlow=1
99-
sink(x.orNull()); // $numTaintFlow=1
100-
sink(x.asSet()); // $numTaintFlow=1
101-
sink(Optional.fromJavaUtil(x.toJavaUtil())); // $numTaintFlow=1
102-
sink(Optional.fromJavaUtil(Optional.toJavaUtil(x))); // $numTaintFlow=1
103-
sink(x.asSet()); // $numTaintFlow=1
104-
sink(Optional.fromNullable(taint())); // $numTaintFlow=1
105-
sink(Optional.absent().or(x)); // $numTaintFlow=1
106-
sink(Optional.absent().or(taint())); // $numTaintFlow=1
107-
sink(Optional.presentInstances(Optional.of(x).asSet())); // $numTaintFlow=1
98+
sink(x.get()); // $numValueFlow=1
99+
sink(x.or("hi")); // $numValueFlow=1
100+
sink(x.orNull()); // $numValueFlow=1
101+
sink(x.asSet().toArray()[0]); // $numValueFlow=1
102+
sink(Optional.fromJavaUtil(x.toJavaUtil()).get()); // $numValueFlow=1
103+
sink(Optional.fromJavaUtil(Optional.toJavaUtil(x)).get()); // $numValueFlow=1
104+
sink(Optional.fromNullable(taint()).get()); // $numValueFlow=1
105+
sink(Optional.absent().or(x).get()); // $numValueFlow=1
106+
sink(Optional.absent().or(taint())); // $numValueFlow=1
107+
sink(Optional.presentInstances(Set.of(x)).iterator().next()); // $numValueFlow=1
108108
}
109109
}

‎java/ql/test/library-tests/frameworks/guava/TestIO.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ void test3() throws IOException {
106106
}
107107

108108
void test4() throws IOException {
109-
sink(Closer.create().register((Closeable) taint())); // $numTaintFlow=1
109+
sink(Closer.create().register((Closeable) taint())); // $numValueFlow=1
110110
sink(new LineReader(rtaint()).readLine()); // $numTaintFlow=1
111111
sink(Files.simplifyPath(staint())); // $numTaintFlow=1
112112
sink(Files.getFileExtension(staint())); // $numTaintFlow=1

‎java/ql/test/library-tests/frameworks/guava/flow.ql

Lines changed: 30 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,20 @@ import java
22
import semmle.code.java.dataflow.TaintTracking
33
import TestUtilities.InlineExpectationsTest
44

5-
class Conf extends TaintTracking::Configuration {
6-
Conf() { this = "qltest:frameworks:guava" }
5+
class TaintFlowConf extends TaintTracking::Configuration {
6+
TaintFlowConf() { this = "qltest:frameworks:guava-taint" }
7+
8+
override predicate isSource(DataFlow::Node n) {
9+
n.asExpr().(MethodAccess).getMethod().hasName("taint")
10+
}
11+
12+
override predicate isSink(DataFlow::Node n) {
13+
exists(MethodAccess ma | ma.getMethod().hasName("sink") | n.asExpr() = ma.getAnArgument())
14+
}
15+
}
16+
17+
class ValueFlowConf extends DataFlow::Configuration {
18+
ValueFlowConf() { this = "qltest:frameworks:guava-value" }
719

820
override predicate isSource(DataFlow::Node n) {
921
n.asExpr().(MethodAccess).getMethod().hasName("taint")
@@ -17,15 +29,28 @@ class Conf extends TaintTracking::Configuration {
1729
class HasFlowTest extends InlineExpectationsTest {
1830
HasFlowTest() { this = "HasFlowTest" }
1931

20-
override string getARelevantTag() { result = "numTaintFlow" }
32+
override string getARelevantTag() { result = ["numTaintFlow", "numValueFlow"] }
2133

2234
override predicate hasActualResult(Location location, string element, string tag, string value) {
2335
tag = "numTaintFlow" and
24-
exists(DataFlow::Node src, DataFlow::Node sink, Conf conf, int num | conf.hasFlow(src, sink) |
36+
exists(DataFlow::Node src, DataFlow::Node sink, TaintFlowConf tconf, int num |
37+
tconf.hasFlow(src, sink)
38+
|
39+
not any(ValueFlowConf vconf).hasFlow(src, sink) and
40+
value = num.toString() and
41+
sink.getLocation() = location and
42+
element = sink.toString() and
43+
num = strictcount(DataFlow::Node src2 | tconf.hasFlow(src2, sink))
44+
)
45+
or
46+
tag = "numValueFlow" and
47+
exists(DataFlow::Node src, DataFlow::Node sink, ValueFlowConf vconf, int num |
48+
vconf.hasFlow(src, sink)
49+
|
2550
value = num.toString() and
2651
sink.getLocation() = location and
2752
element = sink.toString() and
28-
num = strictcount(DataFlow::Node src2 | conf.hasFlow(src2, sink))
53+
num = strictcount(DataFlow::Node src2 | vconf.hasFlow(src2, sink))
2954
)
3055
}
3156
}

0 commit comments

Comments
 (0)