Skip to content

Commit e813257

Browse files
committed
use hardCode
1 parent b0f7453 commit e813257

File tree

6 files changed

+39
-21
lines changed

6 files changed

+39
-21
lines changed

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,14 @@ public void bad1(HttpServletRequest request) {
1919
}
2020
}
2121

22+
@GetMapping(value = "bad2")
23+
public void bad2(HttpServletRequest request) {
24+
String ip = getClientIP();
25+
if (!"127.0.0.1".equals(ip)) {
26+
new Exception("ip illegal");
27+
}
28+
}
29+
2230
@GetMapping(value = "good1")
2331
@ResponseBody
2432
public String good1(HttpServletRequest request) {

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ bypass a ban-list, for example.</p>
1616
<example>
1717

1818
<p>The following examples show the bad case and the good case respectively.
19-
In the <code>bad1</code> method, the client ip the <code>X-Forwarded-For</code> is split into comma-separated values, but the less-trustworthy first one is used. Both of these examples could be deceived by providing a forged HTTP header. The method
19+
In <code>bad1</code> method and <code>bad2</code> method, the client ip the <code>X-Forwarded-For</code> is split into comma-separated values, but the less-trustworthy first one is used. Both of these examples could be deceived by providing a forged HTTP header. The method
2020
<code>good1</code> similarly splits an <code>X-Forwarded-For</code> value, but uses the last, more-trustworthy entry.</p>
2121

2222
<sample src="UseOfLessTrustedSource.java" />

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

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,8 +36,6 @@ class UseOfLessTrustedSourceConfig extends TaintTracking::Configuration {
3636
not aa.getIndexExpr().(CompileTimeConstantExpr).getIntValue() = 0
3737
)
3838
or
39-
node.getType().hasName("Object")
40-
or
4139
node.getType() instanceof PrimitiveType
4240
or
4341
node.getType() instanceof BoxedType

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

Lines changed: 12 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import java
22
import DataFlow
3+
import semmle.code.java.frameworks.Networking
34
import semmle.code.java.security.QueryInjection
45
import experimental.semmle.code.java.Logging
56

@@ -40,14 +41,10 @@ private class CompareSink extends UseOfLessTrustedSink {
4041
ma.getMethod().getNumberOfParameters() = 1 and
4142
(
4243
ma.getArgument(0) = this.asExpr() and
43-
not ma.getQualifier().(CompileTimeConstantExpr).getStringValue().toLowerCase() in [
44-
"", "unknown", ":"
45-
]
44+
ma.getQualifier().(CompileTimeConstantExpr).getStringValue() instanceof PrivateHostName
4645
or
4746
ma.getQualifier() = this.asExpr() and
48-
not ma.getArgument(0).(CompileTimeConstantExpr).getStringValue().toLowerCase() in [
49-
"", "unknown", ":"
50-
]
47+
ma.getArgument(0).(CompileTimeConstantExpr).getStringValue() instanceof PrivateHostName
5148
)
5249
)
5350
or
@@ -56,9 +53,10 @@ private class CompareSink extends UseOfLessTrustedSink {
5653
ma.getMethod().getDeclaringType() instanceof TypeString and
5754
ma.getMethod().getNumberOfParameters() = 1 and
5855
ma.getQualifier() = this.asExpr() and
59-
not ma.getArgument(0).(CompileTimeConstantExpr).getStringValue().toLowerCase() in [
60-
"", "unknown"
61-
]
56+
ma.getAnArgument()
57+
.(CompileTimeConstantExpr)
58+
.getStringValue()
59+
.regexpMatch("^((10\\.((1\\d{2})?|(2[0-4]\\d)?|(25[0-5])?|([1-9]\\d|[0-9])?)(\\.)?)|(192\\.168\\.)|172\\.(1[6789]|2[0-9]|3[01])\\.)((1\\d{2})?|(2[0-4]\\d)?|(25[0-5])?|([1-9]\\d|[0-9])?)(\\.)?((1\\d{2})?|(2[0-4]\\d)?|(25[0-5])?|([1-9]\\d|[0-9])?)$")
6260
)
6361
or
6462
exists(MethodAccess ma |
@@ -68,7 +66,10 @@ private class CompareSink extends UseOfLessTrustedSink {
6866
.hasQualifiedName(["org.apache.commons.lang3", "org.apache.commons.lang"], "StringUtils") and
6967
ma.getMethod().getNumberOfParameters() = 2 and
7068
ma.getAnArgument() = this.asExpr() and
71-
ma.getAnArgument().(CompileTimeConstantExpr).getStringValue() != ""
69+
ma.getAnArgument()
70+
.(CompileTimeConstantExpr)
71+
.getStringValue()
72+
.regexpMatch("^((10\\.((1\\d{2})?|(2[0-4]\\d)?|(25[0-5])?|([1-9]\\d|[0-9])?)(\\.)?)|(192\\.168\\.)|172\\.(1[6789]|2[0-9]|3[01])\\.)((1\\d{2})?|(2[0-4]\\d)?|(25[0-5])?|([1-9]\\d|[0-9])?)(\\.)?((1\\d{2})?|(2[0-4]\\d)?|(25[0-5])?|([1-9]\\d|[0-9])?)$")
7273
)
7374
or
7475
exists(MethodAccess ma |
@@ -78,9 +79,7 @@ private class CompareSink extends UseOfLessTrustedSink {
7879
.hasQualifiedName(["org.apache.commons.lang3", "org.apache.commons.lang"], "StringUtils") and
7980
ma.getMethod().getNumberOfParameters() = 2 and
8081
ma.getAnArgument() = this.asExpr() and
81-
not ma.getAnArgument().(CompileTimeConstantExpr).getStringValue().toLowerCase() in [
82-
"", "unknown", ":"
83-
]
82+
ma.getAnArgument().(CompileTimeConstantExpr).getStringValue() instanceof PrivateHostName
8483
)
8584
}
8685
}
Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,16 @@
11
edges
22
| UseOfLessTrustedSource.java:16:21:16:33 | getClientIP(...) : String | UseOfLessTrustedSource.java:17:37:17:38 | ip |
3-
| UseOfLessTrustedSource.java:37:27:37:62 | getHeader(...) : String | UseOfLessTrustedSource.java:41:16:41:37 | ...[...] : String |
4-
| UseOfLessTrustedSource.java:41:16:41:37 | ...[...] : String | UseOfLessTrustedSource.java:16:21:16:33 | getClientIP(...) : String |
3+
| UseOfLessTrustedSource.java:24:21:24:33 | getClientIP(...) : String | UseOfLessTrustedSource.java:25:33:25:34 | ip |
4+
| UseOfLessTrustedSource.java:45:27:45:62 | getHeader(...) : String | UseOfLessTrustedSource.java:49:16:49:37 | ...[...] : String |
5+
| UseOfLessTrustedSource.java:49:16:49:37 | ...[...] : String | UseOfLessTrustedSource.java:16:21:16:33 | getClientIP(...) : String |
6+
| UseOfLessTrustedSource.java:49:16:49:37 | ...[...] : String | UseOfLessTrustedSource.java:24:21:24:33 | getClientIP(...) : String |
57
nodes
68
| UseOfLessTrustedSource.java:16:21:16:33 | getClientIP(...) : String | semmle.label | getClientIP(...) : String |
79
| UseOfLessTrustedSource.java:17:37:17:38 | ip | semmle.label | ip |
8-
| UseOfLessTrustedSource.java:37:27:37:62 | getHeader(...) : String | semmle.label | getHeader(...) : String |
9-
| UseOfLessTrustedSource.java:41:16:41:37 | ...[...] : String | semmle.label | ...[...] : String |
10+
| UseOfLessTrustedSource.java:24:21:24:33 | getClientIP(...) : String | semmle.label | getClientIP(...) : String |
11+
| UseOfLessTrustedSource.java:25:33:25:34 | ip | semmle.label | ip |
12+
| UseOfLessTrustedSource.java:45:27:45:62 | getHeader(...) : String | semmle.label | getHeader(...) : String |
13+
| UseOfLessTrustedSource.java:49:16:49:37 | ...[...] : String | semmle.label | ...[...] : String |
1014
#select
11-
| UseOfLessTrustedSource.java:17:37:17:38 | ip | UseOfLessTrustedSource.java:37:27:37:62 | getHeader(...) : String | UseOfLessTrustedSource.java:17:37:17:38 | ip | IP address spoofing might include code from $@. | UseOfLessTrustedSource.java:37:27:37:62 | getHeader(...) | this user input |
15+
| UseOfLessTrustedSource.java:17:37:17:38 | ip | UseOfLessTrustedSource.java:45:27:45:62 | getHeader(...) : String | UseOfLessTrustedSource.java:17:37:17:38 | ip | IP address spoofing might include code from $@. | UseOfLessTrustedSource.java:45:27:45:62 | getHeader(...) | this user input |
16+
| UseOfLessTrustedSource.java:25:33:25:34 | ip | UseOfLessTrustedSource.java:45:27:45:62 | getHeader(...) : String | UseOfLessTrustedSource.java:25:33:25:34 | ip | IP address spoofing might include code from $@. | UseOfLessTrustedSource.java:45:27:45:62 | getHeader(...) | this user input |

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,14 @@ public void bad1(HttpServletRequest request) {
1919
}
2020
}
2121

22+
@GetMapping(value = "bad2")
23+
public void bad2(HttpServletRequest request) {
24+
String ip = getClientIP();
25+
if (!"127.0.0.1".equals(ip)) {
26+
new Exception("ip illegal");
27+
}
28+
}
29+
2230
@GetMapping(value = "good1")
2331
@ResponseBody
2432
public String good1(HttpServletRequest request) {

0 commit comments

Comments
 (0)