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

Java: add some comments and minor code reorg

Browse Source
This commit is contained in:
Jami Cogswell
2023-11-30 16:05:59 -05:00
parent 1da1e896cb
commit 5a9d7552b3
2 changed files with 30 additions and 26 deletions
Download Patch File Download Diff File Expand all files Collapse all files

54
java/ql/lib/semmle/code/java/security/UnsafeUrlForward.qll
View File

@@ -1,20 +1,40 @@
/** Provides classes related to unsafe URL forwarding in Java. */
import java
private import semmle.code.java.dataflow.ExternalFlow
private import semmle.code.java.dataflow.FlowSources
private import semmle.code.java.dataflow.StringPrefixes
private import semmle.code.java.frameworks.javaee.ejb.EJBRestrictions
/** A sink for unsafe URL forward vulnerabilities. */
abstract class UnsafeUrlForwardSink extends DataFlow::Node { }
/** A sanitizer for unsafe URL forward vulnerabilities. */
abstract class UnsafeUrlForwardSanitizer extends DataFlow::Node { }
/** A default sink representing methods susceptible to unsafe URL forwarding. */
private class DefaultUnsafeUrlForwardSink extends UnsafeUrlForwardSink {
DefaultUnsafeUrlForwardSink() { sinkNode(this, "url-forward") }
}
/**
* An expression appended (perhaps indirectly) to `"forward:"`, and which
* is reachable from a Spring entry point.
*/
private class SpringUrlForwardSink extends UnsafeUrlForwardSink {
SpringUrlForwardSink() {
// TODO: check if can use MaD "Annotated" for `SpringRequestMappingMethod` or if too complicated for MaD (probably too complicated).
any(SpringRequestMappingMethod sqmm).polyCalls*(this.getEnclosingCallable()) and
this.asExpr() = any(ForwardPrefix fp).getAnAppendedExpression()
}
}
// TODO: should this potentially be "include:" as well? Or does that not work similarly?
private class ForwardPrefix extends InterestingPrefix {
ForwardPrefix() { this.getStringValue() = "forward:" }
override int getOffset() { result = 0 }
}
/** A sanitizer for unsafe URL forward vulnerabilities. */
abstract class UnsafeUrlForwardSanitizer extends DataFlow::Node { }
private class PrimitiveSanitizer extends UnsafeUrlForwardSanitizer {
PrimitiveSanitizer() {
this.getType() instanceof PrimitiveType or
@@ -23,6 +43,11 @@ private class PrimitiveSanitizer extends UnsafeUrlForwardSanitizer {
}
}
// TODO: double-check this sanitizer (and should I switch all "sanitizer" naming to "barrier" instead?)
private class FollowsSanitizingPrefix extends UnsafeUrlForwardSanitizer {
FollowsSanitizingPrefix() { this.asExpr() = any(SanitizingPrefix fp).getAnAppendedExpression() }
}
private class SanitizingPrefix extends InterestingPrefix {
SanitizingPrefix() {
not this.getStringValue().matches("/WEB-INF/%") and
@@ -31,24 +56,3 @@ private class SanitizingPrefix extends InterestingPrefix {
override int getOffset() { result = 0 }
}
private class FollowsSanitizingPrefix extends UnsafeUrlForwardSanitizer {
FollowsSanitizingPrefix() { this.asExpr() = any(SanitizingPrefix fp).getAnAppendedExpression() }
}
private class ForwardPrefix extends InterestingPrefix {
ForwardPrefix() { this.getStringValue() = "forward:" }
override int getOffset() { result = 0 }
}
/**
* An expression appended (perhaps indirectly) to `"forward:"`, and which
* is reachable from a Spring entry point.
*/
private class SpringUrlForwardSink extends UnsafeUrlForwardSink {
SpringUrlForwardSink() {
any(SpringRequestMappingMethod sqmm).polyCalls*(this.getEnclosingCallable()) and
this.asExpr() = any(ForwardPrefix fp).getAnAppendedExpression()
}
}

2
java/ql/lib/semmle/code/java/security/UnsafeUrlForwardQuery.qll
View File

@@ -12,7 +12,7 @@ import semmle.code.java.security.PathSanitizer
module UnsafeUrlForwardFlowConfig implements DataFlow::ConfigSig {
predicate isSource(DataFlow::Node source) {
source instanceof ThreatModelFlowSource and
// TODO: move below logic to class in UnsafeUrlForward.qll?
// TODO: move below logic to class in UnsafeUrlForward.qll? And check exactly why these were excluded.
not exists(MethodCall ma, Method m | ma.getMethod() = m |
(
m instanceof HttpServletRequestGetRequestUriMethod or