hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2026-04-29 10:45:15 +02:00
Code Issues Packages Projects Releases Wiki Activity

Switch to the shared PathSanitizer library

Browse Source
This commit is contained in:
luchua-bc
2022-02-16 16:06:28 +00:00
parent 26e7f3273b
commit f136ea0f6f
2 changed files with 2 additions and 179 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
java/ql/src/experimental/Security/CWE/CWE-552/UnsafeUrlForward.ql
View File

@@ -12,7 +12,7 @@
import java
import UnsafeUrlForward
import semmle.code.java.dataflow.FlowSources
import experimental.semmle.code.java.PathSanitizer
import DataFlow::PathGraph
class UnsafeUrlForwardFlowConfig extends TaintTracking::Configuration {
@@ -35,7 +35,7 @@ class UnsafeUrlForwardFlowConfig extends TaintTracking::Configuration {
override predicate isSanitizer(DataFlow::Node node) { node instanceof UnsafeUrlForwardSanitizer }
override predicate isSanitizerGuard(DataFlow::BarrierGuard guard) {
guard instanceof UnsafeUrlForwardBarrierGuard
guard instanceof PathTraversalBarrierGuard
}
override DataFlow::FlowFeature getAFeature() {

177
java/ql/src/experimental/Security/CWE/CWE-552/UnsafeUrlForward.qll
View File

@@ -1,10 +1,5 @@
import java
import DataFlow
import semmle.code.java.controlflow.Guards
import semmle.code.java.dataflow.FlowSources
import semmle.code.java.frameworks.Servlets
import semmle.code.java.frameworks.spring.SpringWeb
import semmle.code.java.security.RequestForgery
private import semmle.code.java.dataflow.StringPrefixes
/** A sink for unsafe URL forward vulnerabilities. */
@@ -13,9 +8,6 @@ abstract class UnsafeUrlForwardSink extends DataFlow::Node { }
/** A sanitizer for unsafe URL forward vulnerabilities. */
abstract class UnsafeUrlForwardSanitizer extends DataFlow::Node { }
/** A barrier guard that protects against URL forward vulnerabilities. */
abstract class UnsafeUrlForwardBarrierGuard extends DataFlow::BarrierGuard { }
/** An argument to `getRequestDispatcher`. */
private class RequestDispatcherSink extends UnsafeUrlForwardSink {
RequestDispatcherSink() {
@@ -59,175 +51,6 @@ private class FollowsSanitizingPrefix extends UnsafeUrlForwardSanitizer {
FollowsSanitizingPrefix() { this.asExpr() = any(SanitizingPrefix fp).getAnAppendedExpression() }
}
/**
* A guard that considers safe a string being exactly compared to a trusted value.
*/
private class ExactStringPathMatchGuard extends UnsafeUrlForwardBarrierGuard instanceof MethodAccess {
ExactStringPathMatchGuard() {
super.getMethod().getDeclaringType() instanceof TypeString and
super.getMethod().getName() = ["equals", "equalsIgnoreCase"]
}
override predicate checks(Expr e, boolean branch) {
e = super.getQualifier() and
branch = true
}
}
private class AllowListGuard extends Guard instanceof MethodAccess {
AllowListGuard() {
(isStringPartialMatch(this) or isPathPartialMatch(this)) and
not isDisallowedWord(super.getAnArgument())
}
Expr getCheckedExpr() { result = super.getQualifier() }
}
/**
* A guard that considers a path safe because it is checked against an allowlist of partial trusted values.
* This requires additional protection against path traversal, either another guard (`PathTraversalGuard`)
* or a sanitizer (`PathNormalizeSanitizer`), to ensure any internal `..` components are removed from the path.
*/
private class AllowListBarrierGuard extends UnsafeUrlForwardBarrierGuard instanceof AllowListGuard {
override predicate checks(Expr e, boolean branch) {
e = super.getCheckedExpr() and
branch = true and
(
// Either a path normalization sanitizer comes before the guard,
exists(PathNormalizeSanitizer sanitizer | DataFlow::localExprFlow(sanitizer, e))
or
// or a check like `!path.contains("..")` comes before the guard
exists(PathTraversalGuard previousGuard |
DataFlow::localExprFlow(previousGuard.getCheckedExpr(), e) and
previousGuard.controls(this.getBasicBlock().(ConditionBlock), false)
)
)
}
}
/**
* A guard that considers a path safe because it is checked for `..` components, having previously
* been checked for a trusted prefix.
*/
private class DotDotCheckBarrierGuard extends UnsafeUrlForwardBarrierGuard instanceof PathTraversalGuard {
override predicate checks(Expr e, boolean branch) {
e = super.getCheckedExpr() and
branch = false and
// The same value has previously been checked against a list of allowed prefixes:
exists(AllowListGuard previousGuard |
DataFlow::localExprFlow(previousGuard.getCheckedExpr(), e) and
previousGuard.controls(this.getBasicBlock().(ConditionBlock), true)
)
}
}
private class BlockListGuard extends Guard instanceof MethodAccess {
BlockListGuard() {
(isStringPartialMatch(this) or isPathPartialMatch(this)) and
isDisallowedWord(super.getAnArgument())
}
Expr getCheckedExpr() { result = super.getQualifier() }
}
/**
* A guard that considers a string safe because it is checked against a blocklist of known dangerous values.
* This requires a prior check for URL encoding concealing a forbidden value, either a guard (`UrlEncodingGuard`)
* or a sanitizer (`UrlDecodeSanitizer`).
*/
private class BlockListBarrierGuard extends UnsafeUrlForwardBarrierGuard instanceof BlockListGuard {
override predicate checks(Expr e, boolean branch) {
e = super.getCheckedExpr() and
branch = false and
(
// Either `e` has been URL decoded:
exists(UrlDecodeSanitizer sanitizer | DataFlow::localExprFlow(sanitizer, e))
or
// or `e` has previously been checked for URL encoding sequences:
exists(UrlEncodingGuard previousGuard |
DataFlow::localExprFlow(previousGuard.getCheckedExpr(), e) and
previousGuard.controls(this.getBasicBlock(), false)
)
)
}
}
/**
* A guard that considers a string safe because it is checked for URL encoding sequences,
* having previously been checked against a block-list of forbidden values.
*/
private class URLEncodingBarrierGuard extends UnsafeUrlForwardBarrierGuard instanceof UrlEncodingGuard {
override predicate checks(Expr e, boolean branch) {
e = super.getCheckedExpr() and
branch = false and
exists(BlockListGuard previousGuard |
DataFlow::localExprFlow(previousGuard.getCheckedExpr(), e) and
previousGuard.controls(this.getBasicBlock(), false)
)
}
}
/**
* Holds if `ma` is a call to a method that checks a partial string match.
*/
private predicate isStringPartialMatch(MethodAccess ma) {
ma.getMethod().getDeclaringType() instanceof TypeString and
ma.getMethod().getName() =
["contains", "startsWith", "matches", "regionMatches", "indexOf", "lastIndexOf"]
}
/**
* Holds if `ma` is a call to a method of `java.nio.file.Path` that checks a partial path match.
*/
private predicate isPathPartialMatch(MethodAccess ma) {
ma.getMethod().getDeclaringType() instanceof TypePath and
ma.getMethod().getName() = "startsWith"
}
private predicate isDisallowedWord(CompileTimeConstantExpr word) {
word.getStringValue().matches(["%WEB-INF%", "%META-INF%", "%..%"])
}
/** A complementary guard that protects against path traversal, by looking for the literal `..`. */
private class PathTraversalGuard extends Guard instanceof MethodAccess {
Expr checked;
PathTraversalGuard() {
super.getMethod().getDeclaringType() instanceof TypeString and
super.getMethod().hasName(["contains", "indexOf"]) and
super.getAnArgument().(CompileTimeConstantExpr).getStringValue() = ".."
}
Expr getCheckedExpr() { result = super.getQualifier() }
}
/** A complementary sanitizer that protects against path traversal using path normalization. */
private class PathNormalizeSanitizer extends MethodAccess {
PathNormalizeSanitizer() {
this.getMethod().getDeclaringType().hasQualifiedName("java.nio.file", "Path") and
this.getMethod().hasName("normalize")
}
}
/** A complementary guard that protects against double URL encoding, by looking for the literal `%`. */
private class UrlEncodingGuard extends Guard instanceof MethodAccess {
UrlEncodingGuard() {
super.getMethod().getDeclaringType() instanceof TypeString and
super.getMethod().hasName(["contains", "indexOf"]) and
super.getAnArgument().(CompileTimeConstantExpr).getStringValue() = "%"
}
Expr getCheckedExpr() { result = super.getQualifier() }
}
/** A complementary sanitizer that protects against double URL encoding using URL decoding. */
private class UrlDecodeSanitizer extends MethodAccess {
UrlDecodeSanitizer() {
this.getMethod().getDeclaringType().hasQualifiedName("java.net", "URLDecoder") and
this.getMethod().hasName("decode")
}
}
private class ForwardPrefix extends InterestingPrefix {
ForwardPrefix() { this.getStringValue() = "forward:" }