Apply suggestions from code review

Co-authored-by: Taus <tausbn@github.com>
This commit is contained in:
Max Schaefer
2024-01-22 13:17:02 +00:00
committed by Max Schaefer
parent 98178458d0
commit 17e3a45ad7
3 changed files with 23 additions and 10 deletions

View File

@@ -2928,5 +2928,10 @@ module PrivateDjango {
DjangoAllowedUrl() {
this = DataFlow::BarrierGuard<djangoUrlHasAllowedHostAndScheme/3>::getABarrierNode()
}
override predicate sanitizes(UrlRedirect::FlowState state) {
// sanitize all flow states
any()
}
}
}

View File

@@ -55,12 +55,8 @@ module UrlRedirect {
abstract class Sanitizer extends DataFlow::Node {
/**
* Holds if this sanitizer sanitizes flow in the given state.
*
* By default, sanitizers sanitize all flow, but some sanitiziers, for example,
* do not handle untrusted input that contains backslashes, so they only sanitize
* flow in the `NoBackslashes` state.
*/
predicate sanitizes(FlowState state) { any() }
abstract predicate sanitizes(FlowState state);
}
/**
@@ -105,16 +101,23 @@ module UrlRedirect {
string_concat.getRight() = this.asCfgNode()
)
}
override predicate sanitizes(FlowState state) {
// sanitize all flow states
any()
}
}
/**
* A call to replace backslashes with forward slashes or eliminates them
* A call that replaces backslashes with forward slashes or eliminates them
* altogether, considered as a partial sanitizer, as well as an additional
* flow step.
*/
class ReplaceBackslashesSanitizer extends Sanitizer, AdditionalFlowStep, DataFlow::MethodCallNode {
DataFlow::Node receiver;
ReplaceBackslashesSanitizer() {
this.getFunction().(DataFlow::AttrRead).getAttributeName() = "replace" and
this.calls(receiver, "replace") and
this.getArg(0).asExpr().(StrConst).getText() = "\\" and
this.getArg(1).asExpr().(StrConst).getText() in ["/", ""]
}
@@ -124,7 +127,7 @@ module UrlRedirect {
override predicate step(
DataFlow::Node nodeFrom, FlowState stateFrom, DataFlow::Node nodeTo, FlowState stateTo
) {
nodeFrom = this.getObject() and
nodeFrom = receiver and
stateFrom instanceof MayContainBackslashes and
nodeTo = this and
stateTo instanceof NoBackslashes
@@ -134,5 +137,10 @@ module UrlRedirect {
/**
* A comparison with a constant string, considered as a sanitizer-guard.
*/
class StringConstCompareAsSanitizerGuard extends Sanitizer, StringConstCompareBarrier { }
class StringConstCompareAsSanitizerGuard extends Sanitizer, StringConstCompareBarrier {
override predicate sanitizes(FlowState state) {
// sanitize all flow states
any()
}
}
}

View File

@@ -48,7 +48,7 @@ private module UrlRedirectConfig implements DataFlow::StateConfigSig {
predicate isSink(DataFlow::Node sink, FlowState state) {
sink instanceof UrlRedirect::Sink and
state = state
exists(state)
}
predicate isBarrier(DataFlow::Node node, FlowState state) {