Skip to content

Commit 5be9fbb

Browse files
haby0haby0
committed
Remove LogOperationSink and PrintSink
1 parent 407dcea commit 5be9fbb

File tree

11 files changed

+10
-548
lines changed

11 files changed

+10
-548
lines changed

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

Lines changed: 0 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,5 @@
11
import javax.servlet.http.HttpServletRequest;
22
import org.apache.commons.lang3.StringUtils;
3-
import org.slf4j.Logger;
4-
import org.slf4j.LoggerFactory;
53
import org.springframework.beans.factory.annotation.Autowired;
64
import org.springframework.stereotype.Controller;
75
import org.springframework.web.bind.annotation.GetMapping;
@@ -10,46 +8,11 @@
108
@Controller
119
public class UseOfLessTrustedSource {
1210

13-
private static final Logger log = LoggerFactory.getLogger(UseOfLessTrustedSource.class);
14-
1511
@Autowired
1612
private HttpServletRequest request;
1713

1814
@GetMapping(value = "bad1")
1915
public void bad1(HttpServletRequest request) {
20-
String ip = request.getHeader("X-Forwarded-For");
21-
22-
log.debug("getClientIP header X-Forwarded-For:{}", ip);
23-
24-
if (StringUtils.isBlank(ip) || StringUtils.equalsIgnoreCase("unknown", ip)) {
25-
ip = request.getHeader("Proxy-Client-IP");
26-
log.debug("getClientIP header Proxy-Client-IP:{}", ip);
27-
}
28-
if (StringUtils.isBlank(ip) || StringUtils.equalsIgnoreCase("unknown", ip)) {
29-
ip = request.getHeader("WL-Proxy-Client-IP");
30-
log.debug("getClientIP header WL-Proxy-Client-IP:{}", ip);
31-
}
32-
if (StringUtils.isBlank(ip) || StringUtils.equalsIgnoreCase("unknown", ip)) {
33-
ip = request.getHeader("HTTP_CLIENT_IP");
34-
log.debug("getClientIP header HTTP_CLIENT_IP:{}", ip);
35-
}
36-
if (StringUtils.isBlank(ip) || StringUtils.equalsIgnoreCase("unknown", ip)) {
37-
ip = request.getHeader("HTTP_X_FORWARDED_FOR");
38-
log.debug("getClientIP header HTTP_X_FORWARDED_FOR:{}", ip);
39-
}
40-
if (StringUtils.isBlank(ip) || StringUtils.equalsIgnoreCase("unknown", ip)) {
41-
ip = request.getHeader("X-Real-IP");
42-
log.debug("getClientIP header X-Real-IP:{}", ip);
43-
}
44-
if (StringUtils.isBlank(ip) || StringUtils.equalsIgnoreCase("unknown", ip)) {
45-
ip = request.getRemoteAddr();
46-
log.debug("getRemoteAddr IP:{}", ip);
47-
}
48-
System.out.println("client ip is: " + ip);
49-
}
50-
51-
@GetMapping(value = "bad2")
52-
public void bad2(HttpServletRequest request) {
5316
String ip = getClientIP();
5417
if (!StringUtils.startsWith(ip, "192.168.")) {
5518
new Exception("ip illegal");

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +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 is obtained from an HTTP header for local
20-
output and logging. In the <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
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
2120
<code>good1</code> similarly splits an <code>X-Forwarded-For</code> value, but uses the last, more-trustworthy entry.</p>
2221

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

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

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -90,22 +90,6 @@ private class SqlOperationSink extends UseOfLessTrustedSink {
9090
SqlOperationSink() { this instanceof QueryInjectionSink }
9191
}
9292

93-
/** A data flow sink for log operation. */
94-
private class LogOperationSink extends UseOfLessTrustedSink {
95-
LogOperationSink() { exists(LoggingCall lc | lc.getAnArgument() = this.asExpr()) }
96-
}
97-
98-
/** A data flow sink for local output. */
99-
private class PrintSink extends UseOfLessTrustedSink {
100-
PrintSink() {
101-
exists(MethodAccess ma |
102-
ma.getMethod().getName() in ["print", "println"] and
103-
ma.getMethod().getDeclaringType().hasQualifiedName("java.io", ["PrintWriter", "PrintStream"]) and
104-
ma.getAnArgument() = this.asExpr()
105-
)
106-
}
107-
}
108-
10993
/** A method that split string. */
11094
class SplitMethod extends Method {
11195
SplitMethod() {

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

Lines changed: 8 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -1,48 +1,11 @@
11
edges
2-
| UseOfLessTrustedSource.java:20:21:20:56 | getHeader(...) : String | UseOfLessTrustedSource.java:22:60:22:61 | ip |
3-
| UseOfLessTrustedSource.java:20:21:20:56 | getHeader(...) : String | UseOfLessTrustedSource.java:48:28:48:48 | ... + ... |
4-
| UseOfLessTrustedSource.java:25:18:25:53 | getHeader(...) : String | UseOfLessTrustedSource.java:26:64:26:65 | ip |
5-
| UseOfLessTrustedSource.java:25:18:25:53 | getHeader(...) : String | UseOfLessTrustedSource.java:48:28:48:48 | ... + ... |
6-
| UseOfLessTrustedSource.java:29:18:29:56 | getHeader(...) : String | UseOfLessTrustedSource.java:30:67:30:68 | ip |
7-
| UseOfLessTrustedSource.java:29:18:29:56 | getHeader(...) : String | UseOfLessTrustedSource.java:48:28:48:48 | ... + ... |
8-
| UseOfLessTrustedSource.java:33:18:33:52 | getHeader(...) : String | UseOfLessTrustedSource.java:34:63:34:64 | ip |
9-
| UseOfLessTrustedSource.java:33:18:33:52 | getHeader(...) : String | UseOfLessTrustedSource.java:48:28:48:48 | ... + ... |
10-
| UseOfLessTrustedSource.java:37:18:37:58 | getHeader(...) : String | UseOfLessTrustedSource.java:38:69:38:70 | ip |
11-
| UseOfLessTrustedSource.java:37:18:37:58 | getHeader(...) : String | UseOfLessTrustedSource.java:48:28:48:48 | ... + ... |
12-
| UseOfLessTrustedSource.java:41:18:41:47 | getHeader(...) : String | UseOfLessTrustedSource.java:42:58:42:59 | ip |
13-
| 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:37:54:38 | ip |
15-
| UseOfLessTrustedSource.java:74:27:74:62 | getHeader(...) : String | UseOfLessTrustedSource.java:78:16:78:37 | ...[...] : String |
16-
| UseOfLessTrustedSource.java:78:16:78:37 | ...[...] : String | UseOfLessTrustedSource.java:53:21:53:33 | getClientIP(...) : String |
2+
| 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 |
175
nodes
18-
| UseOfLessTrustedSource.java:20:21:20:56 | getHeader(...) : String | semmle.label | getHeader(...) : String |
19-
| UseOfLessTrustedSource.java:22:60:22:61 | ip | semmle.label | ip |
20-
| UseOfLessTrustedSource.java:25:18:25:53 | getHeader(...) : String | semmle.label | getHeader(...) : String |
21-
| UseOfLessTrustedSource.java:26:64:26:65 | ip | semmle.label | ip |
22-
| UseOfLessTrustedSource.java:29:18:29:56 | getHeader(...) : String | semmle.label | getHeader(...) : String |
23-
| UseOfLessTrustedSource.java:30:67:30:68 | ip | semmle.label | ip |
24-
| UseOfLessTrustedSource.java:33:18:33:52 | getHeader(...) : String | semmle.label | getHeader(...) : String |
25-
| UseOfLessTrustedSource.java:34:63:34:64 | ip | semmle.label | ip |
26-
| UseOfLessTrustedSource.java:37:18:37:58 | getHeader(...) : String | semmle.label | getHeader(...) : String |
27-
| UseOfLessTrustedSource.java:38:69:38:70 | ip | semmle.label | ip |
28-
| UseOfLessTrustedSource.java:41:18:41:47 | getHeader(...) : String | semmle.label | getHeader(...) : String |
29-
| UseOfLessTrustedSource.java:42:58:42:59 | ip | semmle.label | ip |
30-
| UseOfLessTrustedSource.java:48:28:48:48 | ... + ... | semmle.label | ... + ... |
31-
| UseOfLessTrustedSource.java:53:21:53:33 | getClientIP(...) : String | semmle.label | getClientIP(...) : String |
32-
| UseOfLessTrustedSource.java:54:37:54:38 | ip | semmle.label | ip |
33-
| UseOfLessTrustedSource.java:74:27:74:62 | getHeader(...) : String | semmle.label | getHeader(...) : String |
34-
| UseOfLessTrustedSource.java:78:16:78:37 | ...[...] : String | semmle.label | ...[...] : String |
6+
| UseOfLessTrustedSource.java:16:21:16:33 | getClientIP(...) : String | semmle.label | getClientIP(...) : String |
7+
| 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 |
3510
#select
36-
| UseOfLessTrustedSource.java:22:60:22:61 | ip | UseOfLessTrustedSource.java:20:21:20:56 | getHeader(...) : String | UseOfLessTrustedSource.java:22:60:22:61 | ip | IP address spoofing might include code from $@. | UseOfLessTrustedSource.java:20:21:20:56 | getHeader(...) | this user input |
37-
| UseOfLessTrustedSource.java:26:64:26:65 | ip | UseOfLessTrustedSource.java:25:18:25:53 | getHeader(...) : String | UseOfLessTrustedSource.java:26:64:26:65 | ip | IP address spoofing might include code from $@. | UseOfLessTrustedSource.java:25:18:25:53 | getHeader(...) | this user input |
38-
| UseOfLessTrustedSource.java:30:67:30:68 | ip | UseOfLessTrustedSource.java:29:18:29:56 | getHeader(...) : String | UseOfLessTrustedSource.java:30:67:30:68 | ip | IP address spoofing might include code from $@. | UseOfLessTrustedSource.java:29:18:29:56 | getHeader(...) | this user input |
39-
| UseOfLessTrustedSource.java:34:63:34:64 | ip | UseOfLessTrustedSource.java:33:18:33:52 | getHeader(...) : String | UseOfLessTrustedSource.java:34:63:34:64 | ip | IP address spoofing might include code from $@. | UseOfLessTrustedSource.java:33:18:33:52 | getHeader(...) | this user input |
40-
| UseOfLessTrustedSource.java:38:69:38:70 | ip | UseOfLessTrustedSource.java:37:18:37:58 | getHeader(...) : String | UseOfLessTrustedSource.java:38:69:38:70 | ip | IP address spoofing might include code from $@. | UseOfLessTrustedSource.java:37:18:37:58 | getHeader(...) | this user input |
41-
| UseOfLessTrustedSource.java:42:58:42:59 | ip | UseOfLessTrustedSource.java:41:18:41:47 | getHeader(...) : String | UseOfLessTrustedSource.java:42:58:42:59 | ip | IP address spoofing might include code from $@. | UseOfLessTrustedSource.java:41:18:41:47 | getHeader(...) | this user input |
42-
| UseOfLessTrustedSource.java:48:28:48:48 | ... + ... | UseOfLessTrustedSource.java:20:21:20:56 | getHeader(...) : String | UseOfLessTrustedSource.java:48:28:48:48 | ... + ... | IP address spoofing might include code from $@. | UseOfLessTrustedSource.java:20:21:20:56 | getHeader(...) | this user input |
43-
| UseOfLessTrustedSource.java:48:28:48:48 | ... + ... | UseOfLessTrustedSource.java:25:18:25:53 | getHeader(...) : String | UseOfLessTrustedSource.java:48:28:48:48 | ... + ... | IP address spoofing might include code from $@. | UseOfLessTrustedSource.java:25:18:25:53 | getHeader(...) | this user input |
44-
| UseOfLessTrustedSource.java:48:28:48:48 | ... + ... | UseOfLessTrustedSource.java:29:18:29:56 | getHeader(...) : String | UseOfLessTrustedSource.java:48:28:48:48 | ... + ... | IP address spoofing might include code from $@. | UseOfLessTrustedSource.java:29:18:29:56 | getHeader(...) | this user input |
45-
| 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 |
46-
| 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 |
47-
| 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: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 |
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 |

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

Lines changed: 0 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,5 @@
11
import javax.servlet.http.HttpServletRequest;
22
import org.apache.commons.lang3.StringUtils;
3-
import org.slf4j.Logger;
4-
import org.slf4j.LoggerFactory;
53
import org.springframework.beans.factory.annotation.Autowired;
64
import org.springframework.stereotype.Controller;
75
import org.springframework.web.bind.annotation.GetMapping;
@@ -10,46 +8,11 @@
108
@Controller
119
public class UseOfLessTrustedSource {
1210

13-
private static final Logger log = LoggerFactory.getLogger(UseOfLessTrustedSource.class);
14-
1511
@Autowired
1612
private HttpServletRequest request;
1713

1814
@GetMapping(value = "bad1")
1915
public void bad1(HttpServletRequest request) {
20-
String ip = request.getHeader("X-Forwarded-For");
21-
22-
log.debug("getClientIP header X-Forwarded-For:{}", ip);
23-
24-
if (StringUtils.isBlank(ip) || StringUtils.equalsIgnoreCase("unknown", ip)) {
25-
ip = request.getHeader("Proxy-Client-IP");
26-
log.debug("getClientIP header Proxy-Client-IP:{}", ip);
27-
}
28-
if (StringUtils.isBlank(ip) || StringUtils.equalsIgnoreCase("unknown", ip)) {
29-
ip = request.getHeader("WL-Proxy-Client-IP");
30-
log.debug("getClientIP header WL-Proxy-Client-IP:{}", ip);
31-
}
32-
if (StringUtils.isBlank(ip) || StringUtils.equalsIgnoreCase("unknown", ip)) {
33-
ip = request.getHeader("HTTP_CLIENT_IP");
34-
log.debug("getClientIP header HTTP_CLIENT_IP:{}", ip);
35-
}
36-
if (StringUtils.isBlank(ip) || StringUtils.equalsIgnoreCase("unknown", ip)) {
37-
ip = request.getHeader("HTTP_X_FORWARDED_FOR");
38-
log.debug("getClientIP header HTTP_X_FORWARDED_FOR:{}", ip);
39-
}
40-
if (StringUtils.isBlank(ip) || StringUtils.equalsIgnoreCase("unknown", ip)) {
41-
ip = request.getHeader("X-Real-IP");
42-
log.debug("getClientIP header X-Real-IP:{}", ip);
43-
}
44-
if (StringUtils.isBlank(ip) || StringUtils.equalsIgnoreCase("unknown", ip)) {
45-
ip = request.getRemoteAddr();
46-
log.debug("getRemoteAddr IP:{}", ip);
47-
}
48-
System.out.println("client ip is: " + ip);
49-
}
50-
51-
@GetMapping(value = "bad2")
52-
public void bad2(HttpServletRequest request) {
5316
String ip = getClientIP();
5417
if (!StringUtils.startsWith(ip, "192.168.")) {
5518
new Exception("ip illegal");

‎java/ql/test/experimental/query-tests/security/CWE-348/options

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
//semmle-extractor-options: --javac-args -cp ${testdir}/../../../../stubs/servlet-api-2.4:${testdir}/../../../../stubs/springframework-5.2.3/:${testdir}/../../../../stubs/slf4j-api-1.6.4/:${testdir}/../../../../stubs/apache-commons-lang3-3.7/
1+
//semmle-extractor-options: --javac-args -cp ${testdir}/../../../../stubs/servlet-api-2.4:${testdir}/../../../../stubs/springframework-5.2.3/:${testdir}/../../../../stubs/apache-commons-lang3-3.7/

‎java/ql/test/stubs/slf4j-api-1.6.4/org/slf4j/Logger.java

Lines changed: 0 additions & 127 deletions
This file was deleted.

‎java/ql/test/stubs/slf4j-api-1.6.4/org/slf4j/LoggerFactory.java

Lines changed: 0 additions & 21 deletions
This file was deleted.

‎java/ql/test/stubs/slf4j-api-1.6.4/org/slf4j/Marker.java

Lines changed: 0 additions & 30 deletions
This file was deleted.

0 commit comments

Comments
 (0)