Skip to content

Commit 4543247

Browse files
committed
delete IfStmt
1 parent 84f00c2 commit 4543247

File tree

3 files changed

+48
-63
lines changed

3 files changed

+48
-63
lines changed

java/ql/src/experimental/Security/CWE/CWE-348/UseOfLessTrustedSource.ql

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -36,14 +36,6 @@ class UseOfLessTrustedSourceConfig extends TaintTracking::Configuration {
3636
not aa.getIndexExpr().(CompileTimeConstantExpr).getIntValue() = 0
3737
)
3838
}
39-
40-
override predicate isAdditionalTaintStep(DataFlow::Node prod, DataFlow::Node succ) {
41-
exists(MethodAccess ma |
42-
ma.getAnArgument() = prod.asExpr() and
43-
ma = succ.asExpr() and
44-
ma.getMethod().getReturnType() instanceof BooleanType
45-
)
46-
}
4739
}
4840

4941
from DataFlow::PathNode source, DataFlow::PathNode sink, UseOfLessTrustedSourceConfig conf

java/ql/src/experimental/Security/CWE/CWE-348/UseOfLessTrustedSourceLib.qll

Lines changed: 45 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -27,66 +27,59 @@ class UseOfLessTrustedSource extends DataFlow::Node {
2727
abstract class UseOfLessTrustedSink extends DataFlow::Node { }
2828

2929
/**
30-
* A data flow sink for the if condition, which does not include the null judgment of the remote client ip address.
30+
* A data flow sink for remote client ip comparison.
3131
*
3232
* For example: `if (!StringUtils.startsWith(ipAddr, "192.168.")){...` determine whether the client ip starts
3333
* with `192.168.`, and the program can be deceived by forging the ip address.
34-
* `if (remoteAddr == null || "".equals(remoteAddr)) {...` judging whether the client ip is a null value,
35-
* it needs to be excluded
3634
*/
37-
private class IfConditionSink extends UseOfLessTrustedSink {
38-
IfConditionSink() {
39-
exists(IfStmt is |
40-
is.getCondition() = this.asExpr() and
35+
private class CompareSink extends UseOfLessTrustedSink {
36+
CompareSink() {
37+
exists(MethodAccess ma |
38+
ma.getMethod().getName() in ["equals", "equalsIgnoreCase"] and
39+
ma.getMethod().getDeclaringType() instanceof TypeString and
40+
ma.getMethod().getNumberOfParameters() = 1 and
4141
(
42-
exists(MethodAccess ma |
43-
ma.getMethod().getName() in ["equals", "equalsIgnoreCase"] and
44-
ma.getMethod().getDeclaringType() instanceof TypeString and
45-
ma.getMethod().getNumberOfParameters() = 1 and
46-
not ma.getQualifier().(CompileTimeConstantExpr).getStringValue().toLowerCase() in [
47-
"", "unknown", ":"
48-
] and
49-
not ma.getArgument(0).(CompileTimeConstantExpr).getStringValue().toLowerCase() in [
50-
"", "unknown", ":"
51-
] and
52-
is.getCondition() = ma.getParent*()
53-
)
54-
or
55-
exists(MethodAccess ma |
56-
ma.getMethod().hasName("contains") and
57-
ma.getMethod().getDeclaringType() instanceof TypeString and
58-
ma.getMethod().getNumberOfParameters() = 1 and
59-
ma.getAnArgument().(CompileTimeConstantExpr).getStringValue().toLowerCase() in [
60-
"", "unknown"
61-
] and
62-
is.getCondition() = ma.getParent*()
63-
)
42+
ma.getArgument(0) = this.asExpr() and
43+
not ma.getQualifier().(CompileTimeConstantExpr).getStringValue().toLowerCase() in [
44+
"", "unknown", ":"
45+
]
6446
or
65-
exists(MethodAccess ma |
66-
ma.getMethod().hasName("startsWith") and
67-
ma.getMethod()
68-
.getDeclaringType()
69-
.hasQualifiedName(["org.apache.commons.lang3", "org.apache.commons.lang"],
70-
"StringUtils") and
71-
ma.getMethod().getNumberOfParameters() = 2 and
72-
ma.getAnArgument().(CompileTimeConstantExpr).getStringValue() != "" and
73-
is.getCondition() = ma.getParent*()
74-
)
75-
or
76-
exists(MethodAccess ma |
77-
ma.getMethod().getName() in ["equals", "equalsIgnoreCase"] and
78-
ma.getMethod()
79-
.getDeclaringType()
80-
.hasQualifiedName(["org.apache.commons.lang3", "org.apache.commons.lang"],
81-
"StringUtils") and
82-
ma.getMethod().getNumberOfParameters() = 2 and
83-
not ma.getAnArgument().(CompileTimeConstantExpr).getStringValue().toLowerCase() in [
84-
"", "unknown", ":"
85-
] and
86-
is.getCondition() = ma.getParent*()
87-
)
47+
ma.getQualifier() = this.asExpr() and
48+
not ma.getArgument(0).(CompileTimeConstantExpr).getStringValue().toLowerCase() in [
49+
"", "unknown", ":"
50+
]
8851
)
8952
)
53+
or
54+
exists(MethodAccess ma |
55+
ma.getMethod().hasName("contains") and
56+
ma.getMethod().getDeclaringType() instanceof TypeString and
57+
ma.getMethod().getNumberOfParameters() = 1 and
58+
ma.getQualifier() = this.asExpr() and
59+
ma.getArgument(0).(CompileTimeConstantExpr).getStringValue().toLowerCase() in ["", "unknown"]
60+
)
61+
or
62+
exists(MethodAccess ma, int i |
63+
ma.getMethod().hasName("startsWith") and
64+
ma.getMethod()
65+
.getDeclaringType()
66+
.hasQualifiedName(["org.apache.commons.lang3", "org.apache.commons.lang"], "StringUtils") and
67+
ma.getMethod().getNumberOfParameters() = 2 and
68+
ma.getArgument(i) = this.asExpr() and
69+
ma.getArgument(1 - i).(CompileTimeConstantExpr).getStringValue() != ""
70+
)
71+
or
72+
exists(MethodAccess ma, int i |
73+
ma.getMethod().getName() in ["equals", "equalsIgnoreCase"] and
74+
ma.getMethod()
75+
.getDeclaringType()
76+
.hasQualifiedName(["org.apache.commons.lang3", "org.apache.commons.lang"], "StringUtils") and
77+
ma.getMethod().getNumberOfParameters() = 2 and
78+
ma.getArgument(i) = this.asExpr() and
79+
not ma.getArgument(1 - i).(CompileTimeConstantExpr).getStringValue().toLowerCase() in [
80+
"", "unknown", ":"
81+
]
82+
)
9083
}
9184
}
9285

java/ql/test/experimental/query-tests/security/CWE-348/UseOfLessTrustedSource.expected

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ edges
1111
| UseOfLessTrustedSource.java:37:18:37:58 | getHeader(...) : String | UseOfLessTrustedSource.java:48:28:48:48 | ... + ... |
1212
| UseOfLessTrustedSource.java:41:18:41:47 | getHeader(...) : String | UseOfLessTrustedSource.java:42:58:42:59 | ip |
1313
| UseOfLessTrustedSource.java:41:18:41:47 | getHeader(...) : String | UseOfLessTrustedSource.java:48:28:48:48 | ... + ... |
14-
| UseOfLessTrustedSource.java:53:21:53:33 | getClientIP(...) : String | UseOfLessTrustedSource.java:54:13:54:51 | !... |
14+
| UseOfLessTrustedSource.java:53:21:53:33 | getClientIP(...) : String | UseOfLessTrustedSource.java:54:37:54:38 | ip |
1515
| UseOfLessTrustedSource.java:74:27:74:62 | getHeader(...) : String | UseOfLessTrustedSource.java:78:16:78:37 | ...[...] : String |
1616
| UseOfLessTrustedSource.java:78:16:78:37 | ...[...] : String | UseOfLessTrustedSource.java:53:21:53:33 | getClientIP(...) : String |
1717
nodes
@@ -29,7 +29,7 @@ nodes
2929
| UseOfLessTrustedSource.java:42:58:42:59 | ip | semmle.label | ip |
3030
| UseOfLessTrustedSource.java:48:28:48:48 | ... + ... | semmle.label | ... + ... |
3131
| UseOfLessTrustedSource.java:53:21:53:33 | getClientIP(...) : String | semmle.label | getClientIP(...) : String |
32-
| UseOfLessTrustedSource.java:54:13:54:51 | !... | semmle.label | !... |
32+
| UseOfLessTrustedSource.java:54:37:54:38 | ip | semmle.label | ip |
3333
| UseOfLessTrustedSource.java:74:27:74:62 | getHeader(...) : String | semmle.label | getHeader(...) : String |
3434
| UseOfLessTrustedSource.java:78:16:78:37 | ...[...] : String | semmle.label | ...[...] : String |
3535
#select
@@ -45,4 +45,4 @@ nodes
4545
| UseOfLessTrustedSource.java:48:28:48:48 | ... + ... | UseOfLessTrustedSource.java:33:18:33:52 | getHeader(...) : String | UseOfLessTrustedSource.java:48:28:48:48 | ... + ... | IP address spoofing might include code from $@. | UseOfLessTrustedSource.java:33:18:33:52 | getHeader(...) | this user input |
4646
| UseOfLessTrustedSource.java:48:28:48:48 | ... + ... | UseOfLessTrustedSource.java:37:18:37:58 | getHeader(...) : String | UseOfLessTrustedSource.java:48:28:48:48 | ... + ... | IP address spoofing might include code from $@. | UseOfLessTrustedSource.java:37:18:37:58 | getHeader(...) | this user input |
4747
| UseOfLessTrustedSource.java:48:28:48:48 | ... + ... | UseOfLessTrustedSource.java:41:18:41:47 | getHeader(...) : String | UseOfLessTrustedSource.java:48:28:48:48 | ... + ... | IP address spoofing might include code from $@. | UseOfLessTrustedSource.java:41:18:41:47 | getHeader(...) | this user input |
48-
| UseOfLessTrustedSource.java:54:13:54:51 | !... | UseOfLessTrustedSource.java:74:27:74:62 | getHeader(...) : String | UseOfLessTrustedSource.java:54:13:54:51 | !... | IP address spoofing might include code from $@. | UseOfLessTrustedSource.java:74:27:74:62 | getHeader(...) | this user input |
48+
| UseOfLessTrustedSource.java:54:37:54:38 | ip | UseOfLessTrustedSource.java:74:27:74:62 | getHeader(...) : String | UseOfLessTrustedSource.java:54:37:54:38 | ip | IP address spoofing might include code from $@. | UseOfLessTrustedSource.java:74:27:74:62 | getHeader(...) | this user input |

0 commit comments

Comments
 (0)