From 8296abcea84e89166608fa603025101c4c8eca47 Mon Sep 17 00:00:00 2001
From: haby0 <ha_yub@163.com>
Date: Mon, 19 Apr 2021 20:59:47 +0800
Subject: [PATCH] Fix Modify the ql query (the qhelp part is not modified).

---
 .../CWE/CWE-348/UseOfLessTrustedSource.java   |  38 +++--
 .../CWE/CWE-348/UseOfLessTrustedSource.qhelp  |   9 +-
 .../CWE/CWE-348/UseOfLessTrustedSource.ql     |  27 ++--
 .../CWE/CWE-348/UseOfLessTrustedSourceLib.qll | 146 +++++++++++-------
 .../CWE-348/UseOfLessTrustedSource.expected   |  56 +++++--
 .../CWE-348/UseOfLessTrustedSource.java       |  38 +++--
 .../query-tests/security/CWE-348/options      |   3 +-
 .../beans/factory/annotation/Autowired.java   |  14 ++
 8 files changed, 213 insertions(+), 118 deletions(-)
 create mode 100644 java/ql/test/stubs/springframework-5.2.3/org/springframework/beans/factory/annotation/Autowired.java

diff --git a/java/ql/src/experimental/Security/CWE/CWE-348/UseOfLessTrustedSource.java b/java/ql/src/experimental/Security/CWE/CWE-348/UseOfLessTrustedSource.java
index d338134ed3e..22d7100223c 100644
--- a/java/ql/src/experimental/Security/CWE/CWE-348/UseOfLessTrustedSource.java
+++ b/java/ql/src/experimental/Security/CWE/CWE-348/UseOfLessTrustedSource.java
@@ -2,6 +2,7 @@ import javax.servlet.http.HttpServletRequest;
 import org.apache.commons.lang3.StringUtils;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
+import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.stereotype.Controller;
 import org.springframework.web.bind.annotation.GetMapping;
 import org.springframework.web.bind.annotation.ResponseBody;
@@ -11,24 +12,13 @@ public class UseOfLessTrustedSource {
 
     private static final Logger log = LoggerFactory.getLogger(UseOfLessTrustedSource.class);
 
-    @GetMapping(value = "bad1")
-    @ResponseBody
-    public String bad1(HttpServletRequest request) {
-        String remoteAddr = "";
-        if (request != null) {
-            remoteAddr = request.getHeader("X-FORWARDED-FOR");
-            remoteAddr = remoteAddr.split(",")[0];
-            if (remoteAddr == null || "".equals(remoteAddr)) {
-                remoteAddr = request.getRemoteAddr();
-            }
-        }
-        return remoteAddr;
-    }
+    @Autowired
+    private HttpServletRequest request;
 
-    @GetMapping(value = "bad2")
-    public void bad2(HttpServletRequest request) {
+    @GetMapping(value = "bad1")
+    public void bad1(HttpServletRequest request) {
         String ip = request.getHeader("X-Forwarded-For");
-        
+
         log.debug("getClientIP header X-Forwarded-For:{}", ip);
 
         if (StringUtils.isBlank(ip) || StringUtils.equalsIgnoreCase("unknown", ip)) {
@@ -58,6 +48,14 @@ public class UseOfLessTrustedSource {
         System.out.println("client ip is: " + ip);
     }
 
+    @GetMapping(value = "bad2")
+    public void bad2(HttpServletRequest request) {
+        String ip = getClientIP();
+        if (!StringUtils.startsWith(ip, "192.168.")) {
+            new Exception("ip illegal");
+        }
+    }
+
     @GetMapping(value = "good1")
     @ResponseBody
     public String good1(HttpServletRequest request) {
@@ -71,4 +69,12 @@ public class UseOfLessTrustedSource {
         }
         return remoteAddr;
     }
+
+    protected String getClientIP() {
+        String xfHeader = request.getHeader("X-Forwarded-For");
+        if (xfHeader == null) {
+            return request.getRemoteAddr();
+        }
+        return xfHeader.split(",")[0];
+    }
 }
\ No newline at end of file
diff --git a/java/ql/src/experimental/Security/CWE/CWE-348/UseOfLessTrustedSource.qhelp b/java/ql/src/experimental/Security/CWE/CWE-348/UseOfLessTrustedSource.qhelp
index 1c3a4485cbd..6b2eb2e7eed 100644
--- a/java/ql/src/experimental/Security/CWE/CWE-348/UseOfLessTrustedSource.qhelp
+++ b/java/ql/src/experimental/Security/CWE/CWE-348/UseOfLessTrustedSource.qhelp
@@ -24,14 +24,11 @@ and get the last value of the split array.</p>
 </example>
 <references>
 
-<li>Prevent IP address spoofing with X-Forwarded-For header when using AWS ELB and Clojure Ring: 
-<a href="https://www.dennis-schneider.com/blog/prevent-ip-address-spoofing-with-x-forwarded-for-header-and-aws-elb-in-clojure-ring/">
-Prevent IP address spoofing with...</a>
+<li>Dennis Schneider: <a href="https://www.dennis-schneider.com/blog/prevent-ip-address-spoofing-with-x-forwarded-for-header-and-aws-elb-in-clojure-ring/">
+Prevent IP address spoofing with X-Forwarded-For header when using AWS ELB and Clojure Ring</a>
 </li>
 
-<li>Security Rule Zero: A Warning about X-Forwarded-For: 
-<a href="https://www.f5.com/company/blog/security-rule-zero-a-warning-about-x-forwarded-for">
-Security Rule Zero: A Warning about X-Forwarded-For</a>
+<li>Security Rule Zero: <a href="https://www.f5.com/company/blog/security-rule-zero-a-warning-about-x-forwarded-for">A Warning about X-Forwarded-For</a>
 </li>
 
 </references>
diff --git a/java/ql/src/experimental/Security/CWE/CWE-348/UseOfLessTrustedSource.ql b/java/ql/src/experimental/Security/CWE/CWE-348/UseOfLessTrustedSource.ql
index 15d665d37ab..3c3feeb8f03 100644
--- a/java/ql/src/experimental/Security/CWE/CWE-348/UseOfLessTrustedSource.ql
+++ b/java/ql/src/experimental/Security/CWE/CWE-348/UseOfLessTrustedSource.ql
@@ -1,20 +1,23 @@
 /**
- * @name X-Forwarded-For spoofing
+ * @name IP address spoofing
  * @description The software obtains the client ip through `X-Forwarded-For`,
  *              and the attacker can modify the value of `X-Forwarded-For` to forge the ip.
  * @kind path-problem
  * @problem.severity error
  * @precision high
- * @id java/use-of-less-trusted-source
+ * @id java/ip-address-spoofing
  * @tags security
  *       external/cwe/cwe-348
  */
 
 import java
 import UseOfLessTrustedSourceLib
+import semmle.code.java.dataflow.DataFlow2
+import semmle.code.java.dataflow.TaintTracking2
 import semmle.code.java.dataflow.FlowSources
 import DataFlow::PathGraph
 
+/** Taint-tracking configuration tracing flow from get method request sources to output jsonp data. */
 class UseOfLessTrustedSourceConfig extends TaintTracking::Configuration {
   UseOfLessTrustedSourceConfig() { this = "UseOfLessTrustedSourceConfig" }
 
@@ -23,22 +26,26 @@ class UseOfLessTrustedSourceConfig extends TaintTracking::Configuration {
   override predicate isSink(DataFlow::Node sink) { sink instanceof UseOfLessTrustedSink }
 
   /**
-   * When using `,` split request data and not taking the first value of
-   *  the array, it is considered as `good`.
+   * When using `,` split request data and not taking the first value of the array, it is considered as `good`.
    */
   override predicate isSanitizer(DataFlow::Node node) {
     exists(ArrayAccess aa, MethodAccess ma | aa.getArray() = ma |
       ma.getQualifier() = node.asExpr() and
       ma.getMethod() instanceof SplitMethod and
-      not aa.getIndexExpr().toString() = "0"
+      not aa.getIndexExpr().(CompileTimeConstantExpr).getIntValue() = 0
+    )
+  }
+
+  override predicate isAdditionalTaintStep(DataFlow::Node prod, DataFlow::Node succ) {
+    exists(MethodAccess ma |
+      ma.getAnArgument() = prod.asExpr() and
+      ma = succ.asExpr() and
+      ma.getMethod().getReturnType() instanceof BooleanType
     )
   }
 }
 
 from DataFlow::PathNode source, DataFlow::PathNode sink, UseOfLessTrustedSourceConfig conf
-where
-  conf.hasFlowPath(source, sink) and
-  source.getNode().getEnclosingCallable() = sink.getNode().getEnclosingCallable() and
-  xffIsFirstGet(source.getNode())
-select sink.getNode(), source, sink, "X-Forwarded-For spoofing might include code from $@.",
+where conf.hasFlowPath(source, sink)
+select sink.getNode(), source, sink, "IP address spoofing might include code from $@.",
   source.getNode(), "this user input"
diff --git a/java/ql/src/experimental/Security/CWE/CWE-348/UseOfLessTrustedSourceLib.qll b/java/ql/src/experimental/Security/CWE/CWE-348/UseOfLessTrustedSourceLib.qll
index 0598f843840..ac72be64f5d 100644
--- a/java/ql/src/experimental/Security/CWE/CWE-348/UseOfLessTrustedSourceLib.qll
+++ b/java/ql/src/experimental/Security/CWE/CWE-348/UseOfLessTrustedSourceLib.qll
@@ -1,24 +1,105 @@
 import java
 import DataFlow
-import semmle.code.java.dataflow.DataFlow
+import semmle.code.java.dataflow.TaintTracking2
+import semmle.code.java.security.QueryInjection
 import experimental.semmle.code.java.Logging
 
-/** A data flow source of the value of the `x-forwarded-for` field in the `header`. */
+/**
+ * A data flow source of the client ip obtained according to the remote endpoint identifier specified
+ * in the header (`X-Forwarded-For`, `X-Real-IP`, `Proxy-Client-IP`, etc.).
+ *
+ * For example: `ServletRequest.getHeader("X-Forwarded-For")`.
+ */
 class UseOfLessTrustedSource extends DataFlow::Node {
   UseOfLessTrustedSource() {
     exists(MethodAccess ma |
       ma.getMethod().hasName("getHeader") and
-      ma.getArgument(0).toString().toLowerCase() = "\"x-forwarded-for\"" and
+      ma.getArgument(0).(CompileTimeConstantExpr).getStringValue().toLowerCase() in [
+          "x-forwarded-for", "x-real-ip", "proxy-client-ip", "wl-proxy-client-ip",
+          "http_x_forwarded_for", "http_x_forwarded", "http_x_cluster_client_ip", "http_client_ip",
+          "http_forwarded_for", "http_forwarded", "http_via", "remote_addr"
+        ] and
       ma = this.asExpr()
     )
   }
 }
 
-/** A data flow sink of method return or log output or local print. */
-class UseOfLessTrustedSink extends DataFlow::Node {
-  UseOfLessTrustedSink() {
-    exists(ReturnStmt rs | rs.getResult() = this.asExpr())
-    or
+/** A data flow sink for ip address forgery vulnerabilities. */
+abstract class UseOfLessTrustedSink extends DataFlow::Node { }
+
+/**
+ * A data flow sink for the if condition, which does not include the null judgment of the remote client ip address.
+ *
+ * For example: `if (!StringUtils.startsWith(ipAddr, "192.168.")){...` determine whether the client ip starts
+ * with `192.168.`, and the program can be deceived by forging the ip address.
+ * `if (remoteAddr == null || "".equals(remoteAddr)) {...` judging whether the client ip is a null value,
+ * it needs to be excluded
+ */
+private class IfConditionSink extends UseOfLessTrustedSink {
+  IfConditionSink() {
+    exists(IfStmt is |
+      is.getCondition() = this.asExpr() and
+      not exists(EQExpr eqe |
+        eqe.getAnOperand() instanceof NullLiteral and
+        is.getCondition() = eqe.getParent*()
+      ) and
+      not exists(NEExpr nee |
+        nee.getAnOperand() instanceof NullLiteral and
+        is.getCondition() = nee.getParent*()
+      ) and
+      not exists(MethodAccess ma |
+        ma.getMethod().hasName("equals") and
+        ma.getMethod().getNumberOfParameters() = 1 and
+        (
+          ma.getQualifier().(CompileTimeConstantExpr).getStringValue() = "" or
+          ma.getArgument(0).(CompileTimeConstantExpr).getStringValue() = ""
+        ) and
+        is.getCondition() = ma.getParent*()
+      ) and
+      not exists(MethodAccess ma |
+        ma.getMethod().hasName("equalsIgnoreCase") and
+        ma.getMethod().getNumberOfParameters() = 1 and
+        (
+          ma.getQualifier().(CompileTimeConstantExpr).getStringValue() = "unknown" or
+          ma.getArgument(0).(CompileTimeConstantExpr).getStringValue() = "unknown"
+        ) and
+        is.getCondition() = ma.getParent*()
+      ) and
+      not exists(MethodAccess ma |
+        ma.getMethod().getName() in ["isEmpty", "isNotEmpty"] and
+        ma.getMethod().getNumberOfParameters() = 1 and
+        is.getCondition() = ma.getParent*()
+      ) and
+      not exists(MethodAccess ma |
+        (
+          ma.getMethod().hasQualifiedName("org.apache.commons.lang3", "StringUtils", "isBlank") or
+          ma.getMethod().hasQualifiedName("org.apache.commons.lang3", "StringUtils", "isNotBlank")
+        ) and
+        is.getCondition() = ma.getParent*()
+      ) and
+      not exists(MethodAccess ma |
+        ma.getMethod()
+            .hasQualifiedName("org.apache.commons.lang3", "StringUtils", "equalsIgnoreCase") and
+        ma.getAnArgument().(CompileTimeConstantExpr).getStringValue() = "unknown" and
+        is.getCondition() = ma.getParent*()
+      )
+    )
+  }
+}
+
+/** A data flow sink for sql operation. */
+private class SqlOperationSink extends UseOfLessTrustedSink {
+  SqlOperationSink() { this instanceof QueryInjectionSink }
+}
+
+/** A data flow sink for log operation. */
+private class LogOperationSink extends UseOfLessTrustedSink {
+  LogOperationSink() { exists(LoggingCall lc | lc.getAnArgument() = this.asExpr()) }
+}
+
+/** A data flow sink for local output. */
+private class PrintSink extends UseOfLessTrustedSink {
+  PrintSink() {
     exists(MethodAccess ma |
       ma.getMethod().getName() in ["print", "println"] and
       (
@@ -27,8 +108,6 @@ class UseOfLessTrustedSink extends DataFlow::Node {
       ) and
       ma.getAnArgument() = this.asExpr()
     )
-    or
-    exists(LoggingCall lc | lc.getAnArgument() = this.asExpr())
   }
 }
 
@@ -39,50 +118,3 @@ class SplitMethod extends Method {
     this.hasQualifiedName("java.lang", "String", "split")
   }
 }
-
-/**
- * A call to the ServletRequest.getHeader method and the argument are
- * `wl-proxy-client-ip`/`proxy-client-ip`/`http_client_ip`/`http_x_forwarded_for`/`x-real-ip`.
- */
-class HeaderIpCall extends MethodAccess {
-  HeaderIpCall() {
-    this.getMethod().hasName("getHeader") and
-    this.getMethod()
-        .getDeclaringType()
-        .getASupertype*()
-        .hasQualifiedName("javax.servlet", "ServletRequest") and
-    this.getArgument(0).toString().toLowerCase() in [
-        "\"wl-proxy-client-ip\"", "\"proxy-client-ip\"", "\"http_client_ip\"",
-        "\"http_x_forwarded_for\"", "\"x-real-ip\""
-      ]
-  }
-}
-
-/** A call to `ServletRequest.getRemoteAddr` method. */
-class RemoteAddrCall extends MethodAccess {
-  RemoteAddrCall() {
-    this.getMethod().hasName("getRemoteAddr") and
-    this.getMethod()
-        .getDeclaringType()
-        .getASupertype*()
-        .hasQualifiedName("javax.servlet", "ServletRequest")
-  }
-}
-
-/** The first one in the method to get the ip value through `x-forwarded-for`. */
-predicate xffIsFirstGet(Node node) {
-  exists(HeaderIpCall hic |
-    node.getEnclosingCallable() = hic.getEnclosingCallable() and
-    node.getLocation().getEndLine() < hic.getLocation().getEndLine()
-  )
-  or
-  exists(RemoteAddrCall rac |
-    node.getEnclosingCallable() = rac.getEnclosingCallable() and
-    node.getLocation().getEndLine() < rac.getLocation().getEndLine()
-  )
-  or
-  not exists(HeaderIpCall hic, RemoteAddrCall rac |
-    node.getEnclosingCallable() = hic.getEnclosingCallable() and
-    node.getEnclosingCallable() = rac.getEnclosingCallable()
-  )
-}
diff --git a/java/ql/test/experimental/query-tests/security/CWE-348/UseOfLessTrustedSource.expected b/java/ql/test/experimental/query-tests/security/CWE-348/UseOfLessTrustedSource.expected
index a71ad9629ce..65bbc8d218b 100644
--- a/java/ql/test/experimental/query-tests/security/CWE-348/UseOfLessTrustedSource.expected
+++ b/java/ql/test/experimental/query-tests/security/CWE-348/UseOfLessTrustedSource.expected
@@ -1,14 +1,48 @@
 edges
-| UseOfLessTrustedSource.java:19:26:19:61 | getHeader(...) : String | UseOfLessTrustedSource.java:25:16:25:25 | remoteAddr |
-| UseOfLessTrustedSource.java:30:21:30:56 | getHeader(...) : String | UseOfLessTrustedSource.java:32:60:32:61 | ip |
-| UseOfLessTrustedSource.java:30:21:30:56 | getHeader(...) : String | UseOfLessTrustedSource.java:58:28:58:48 | ... + ... |
+| UseOfLessTrustedSource.java:20:21:20:56 | getHeader(...) : String | UseOfLessTrustedSource.java:22:60:22:61 | ip |
+| UseOfLessTrustedSource.java:20:21:20:56 | getHeader(...) : String | UseOfLessTrustedSource.java:48:28:48:48 | ... + ... |
+| UseOfLessTrustedSource.java:25:18:25:53 | getHeader(...) : String | UseOfLessTrustedSource.java:26:64:26:65 | ip |
+| UseOfLessTrustedSource.java:25:18:25:53 | getHeader(...) : String | UseOfLessTrustedSource.java:48:28:48:48 | ... + ... |
+| UseOfLessTrustedSource.java:29:18:29:56 | getHeader(...) : String | UseOfLessTrustedSource.java:30:67:30:68 | ip |
+| UseOfLessTrustedSource.java:29:18:29:56 | getHeader(...) : String | UseOfLessTrustedSource.java:48:28:48:48 | ... + ... |
+| UseOfLessTrustedSource.java:33:18:33:52 | getHeader(...) : String | UseOfLessTrustedSource.java:34:63:34:64 | ip |
+| UseOfLessTrustedSource.java:33:18:33:52 | getHeader(...) : String | UseOfLessTrustedSource.java:48:28:48:48 | ... + ... |
+| UseOfLessTrustedSource.java:37:18:37:58 | getHeader(...) : String | UseOfLessTrustedSource.java:38:69:38:70 | ip |
+| UseOfLessTrustedSource.java:37:18:37:58 | getHeader(...) : String | UseOfLessTrustedSource.java:48:28:48:48 | ... + ... |
+| UseOfLessTrustedSource.java:41:18:41:47 | getHeader(...) : String | UseOfLessTrustedSource.java:42:58:42:59 | ip |
+| UseOfLessTrustedSource.java:41:18:41:47 | getHeader(...) : String | UseOfLessTrustedSource.java:48:28:48:48 | ... + ... |
+| UseOfLessTrustedSource.java:53:21:53:33 | getClientIP(...) : String | UseOfLessTrustedSource.java:54:13:54:51 | !... |
+| UseOfLessTrustedSource.java:74:27:74:62 | getHeader(...) : String | UseOfLessTrustedSource.java:78:16:78:37 | ...[...] : String |
+| UseOfLessTrustedSource.java:78:16:78:37 | ...[...] : String | UseOfLessTrustedSource.java:53:21:53:33 | getClientIP(...) : String |
 nodes
-| UseOfLessTrustedSource.java:19:26:19:61 | getHeader(...) : String | semmle.label | getHeader(...) : String |
-| UseOfLessTrustedSource.java:25:16:25:25 | remoteAddr | semmle.label | remoteAddr |
-| UseOfLessTrustedSource.java:30:21:30:56 | getHeader(...) : String | semmle.label | getHeader(...) : String |
-| UseOfLessTrustedSource.java:32:60:32:61 | ip | semmle.label | ip |
-| UseOfLessTrustedSource.java:58:28:58:48 | ... + ... | semmle.label | ... + ... |
+| UseOfLessTrustedSource.java:20:21:20:56 | getHeader(...) : String | semmle.label | getHeader(...) : String |
+| UseOfLessTrustedSource.java:22:60:22:61 | ip | semmle.label | ip |
+| UseOfLessTrustedSource.java:25:18:25:53 | getHeader(...) : String | semmle.label | getHeader(...) : String |
+| UseOfLessTrustedSource.java:26:64:26:65 | ip | semmle.label | ip |
+| UseOfLessTrustedSource.java:29:18:29:56 | getHeader(...) : String | semmle.label | getHeader(...) : String |
+| UseOfLessTrustedSource.java:30:67:30:68 | ip | semmle.label | ip |
+| UseOfLessTrustedSource.java:33:18:33:52 | getHeader(...) : String | semmle.label | getHeader(...) : String |
+| UseOfLessTrustedSource.java:34:63:34:64 | ip | semmle.label | ip |
+| UseOfLessTrustedSource.java:37:18:37:58 | getHeader(...) : String | semmle.label | getHeader(...) : String |
+| UseOfLessTrustedSource.java:38:69:38:70 | ip | semmle.label | ip |
+| UseOfLessTrustedSource.java:41:18:41:47 | getHeader(...) : String | semmle.label | getHeader(...) : String |
+| UseOfLessTrustedSource.java:42:58:42:59 | ip | semmle.label | ip |
+| UseOfLessTrustedSource.java:48:28:48:48 | ... + ... | semmle.label | ... + ... |
+| UseOfLessTrustedSource.java:53:21:53:33 | getClientIP(...) : String | semmle.label | getClientIP(...) : String |
+| UseOfLessTrustedSource.java:54:13:54:51 | !... | semmle.label | !... |
+| UseOfLessTrustedSource.java:74:27:74:62 | getHeader(...) : String | semmle.label | getHeader(...) : String |
+| UseOfLessTrustedSource.java:78:16:78:37 | ...[...] : String | semmle.label | ...[...] : String |
 #select
-| UseOfLessTrustedSource.java:25:16:25:25 | remoteAddr | UseOfLessTrustedSource.java:19:26:19:61 | getHeader(...) : String | UseOfLessTrustedSource.java:25:16:25:25 | remoteAddr | X-Forwarded-For spoofing might include code from $@. | UseOfLessTrustedSource.java:19:26:19:61 | getHeader(...) | this user input |
-| UseOfLessTrustedSource.java:32:60:32:61 | ip | UseOfLessTrustedSource.java:30:21:30:56 | getHeader(...) : String | UseOfLessTrustedSource.java:32:60:32:61 | ip | X-Forwarded-For spoofing might include code from $@. | UseOfLessTrustedSource.java:30:21:30:56 | getHeader(...) | this user input |
-| UseOfLessTrustedSource.java:58:28:58:48 | ... + ... | UseOfLessTrustedSource.java:30:21:30:56 | getHeader(...) : String | UseOfLessTrustedSource.java:58:28:58:48 | ... + ... | X-Forwarded-For spoofing might include code from $@. | UseOfLessTrustedSource.java:30:21:30:56 | getHeader(...) | this user input |
+| 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 |
+| 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 |
+| 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 |
+| 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 |
+| 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 |
+| 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 |
+| 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 |
+| 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 |
+| 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 |
+| 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 |
+| 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 |
+| 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 |
+| 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 |
diff --git a/java/ql/test/experimental/query-tests/security/CWE-348/UseOfLessTrustedSource.java b/java/ql/test/experimental/query-tests/security/CWE-348/UseOfLessTrustedSource.java
index d338134ed3e..22d7100223c 100644
--- a/java/ql/test/experimental/query-tests/security/CWE-348/UseOfLessTrustedSource.java
+++ b/java/ql/test/experimental/query-tests/security/CWE-348/UseOfLessTrustedSource.java
@@ -2,6 +2,7 @@ import javax.servlet.http.HttpServletRequest;
 import org.apache.commons.lang3.StringUtils;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
+import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.stereotype.Controller;
 import org.springframework.web.bind.annotation.GetMapping;
 import org.springframework.web.bind.annotation.ResponseBody;
@@ -11,24 +12,13 @@ public class UseOfLessTrustedSource {
 
     private static final Logger log = LoggerFactory.getLogger(UseOfLessTrustedSource.class);
 
-    @GetMapping(value = "bad1")
-    @ResponseBody
-    public String bad1(HttpServletRequest request) {
-        String remoteAddr = "";
-        if (request != null) {
-            remoteAddr = request.getHeader("X-FORWARDED-FOR");
-            remoteAddr = remoteAddr.split(",")[0];
-            if (remoteAddr == null || "".equals(remoteAddr)) {
-                remoteAddr = request.getRemoteAddr();
-            }
-        }
-        return remoteAddr;
-    }
+    @Autowired
+    private HttpServletRequest request;
 
-    @GetMapping(value = "bad2")
-    public void bad2(HttpServletRequest request) {
+    @GetMapping(value = "bad1")
+    public void bad1(HttpServletRequest request) {
         String ip = request.getHeader("X-Forwarded-For");
-        
+
         log.debug("getClientIP header X-Forwarded-For:{}", ip);
 
         if (StringUtils.isBlank(ip) || StringUtils.equalsIgnoreCase("unknown", ip)) {
@@ -58,6 +48,14 @@ public class UseOfLessTrustedSource {
         System.out.println("client ip is: " + ip);
     }
 
+    @GetMapping(value = "bad2")
+    public void bad2(HttpServletRequest request) {
+        String ip = getClientIP();
+        if (!StringUtils.startsWith(ip, "192.168.")) {
+            new Exception("ip illegal");
+        }
+    }
+
     @GetMapping(value = "good1")
     @ResponseBody
     public String good1(HttpServletRequest request) {
@@ -71,4 +69,12 @@ public class UseOfLessTrustedSource {
         }
         return remoteAddr;
     }
+
+    protected String getClientIP() {
+        String xfHeader = request.getHeader("X-Forwarded-For");
+        if (xfHeader == null) {
+            return request.getRemoteAddr();
+        }
+        return xfHeader.split(",")[0];
+    }
 }
\ No newline at end of file
diff --git a/java/ql/test/experimental/query-tests/security/CWE-348/options b/java/ql/test/experimental/query-tests/security/CWE-348/options
index 40bcee8c133..7edf075ba8e 100644
--- a/java/ql/test/experimental/query-tests/security/CWE-348/options
+++ b/java/ql/test/experimental/query-tests/security/CWE-348/options
@@ -1,2 +1 @@
-//semmle-extractor-options: --javac-args -cp ${testdir}/../../../../stubs/servlet-api-2.4:${testdir}/../../../../stubs/springframework-5.2.3/:${tes
-tdir}/../../../../stubs/slf4j-api-1.6.4/:${testdir}/../../../../stubs/apache-commons-lang3-3.7/
\ No newline at end of file
+//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/
\ No newline at end of file
diff --git a/java/ql/test/stubs/springframework-5.2.3/org/springframework/beans/factory/annotation/Autowired.java b/java/ql/test/stubs/springframework-5.2.3/org/springframework/beans/factory/annotation/Autowired.java
new file mode 100644
index 00000000000..8fcef979360
--- /dev/null
+++ b/java/ql/test/stubs/springframework-5.2.3/org/springframework/beans/factory/annotation/Autowired.java
@@ -0,0 +1,14 @@
+package org.springframework.beans.factory.annotation;
+
+import java.lang.annotation.Documented;
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+import java.lang.annotation.Target;
+
+@Target({ElementType.CONSTRUCTOR, ElementType.METHOD, ElementType.PARAMETER, ElementType.FIELD, ElementType.ANNOTATION_TYPE})
+@Retention(RetentionPolicy.RUNTIME)
+@Documented
+public @interface Autowired {
+    boolean required() default true;
+}
\ No newline at end of file