hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2026-02-11 04:31:05 +01:00
Code Issues Packages Projects Releases Wiki Activity

Fix Modify the ql query (the qhelp part is not modified).

Browse Source
This commit is contained in:
haby0
2021-04-19 20:59:47 +08:00
parent 23b508c5e7
commit 8296abcea8
8 changed files with 213 additions and 118 deletions
Download Patch File Download Diff File Expand all files Collapse all files

38
java/ql/src/experimental/Security/CWE/CWE-348/UseOfLessTrustedSource.java
View File

@@ -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];
}
}

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

@@ -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>

27
java/ql/src/experimental/Security/CWE/CWE-348/UseOfLessTrustedSource.ql
View File

@@ -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"

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

@@ -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()
)
}