mirror of
https://github.com/github/codeql.git
synced 2026-04-25 16:55:19 +02:00
C#: Re-factor the InappropriateEncoding query to use the new API.
This commit is contained in:
@@ -20,57 +20,47 @@ import semmle.code.csharp.security.dataflow.SqlInjectionQuery as SqlInjection
|
||||
import semmle.code.csharp.security.dataflow.flowsinks.Html
|
||||
import semmle.code.csharp.security.dataflow.UrlRedirectQuery as UrlRedirect
|
||||
import semmle.code.csharp.security.Sanitizers
|
||||
import semmle.code.csharp.dataflow.DataFlow2::DataFlow2
|
||||
import semmle.code.csharp.dataflow.DataFlow2::DataFlow2::PathGraph
|
||||
import semmle.code.csharp.dataflow.TaintTracking2
|
||||
import EncodingConfigurations::Flow::PathGraph
|
||||
|
||||
signature module EncodingConfigSig {
|
||||
/** Holds if `n` is a node whose value must be encoded. */
|
||||
predicate requiresEncoding(DataFlow::Node n);
|
||||
|
||||
/** Holds if `e` is a possible valid encoded value. */
|
||||
predicate isPossibleEncodedValue(Expr e);
|
||||
}
|
||||
|
||||
/**
|
||||
* A configuration for specifying expressions that must be
|
||||
* encoded, along with a set of potential valid encoded values.
|
||||
*/
|
||||
abstract class RequiresEncodingConfiguration extends TaintTracking2::Configuration {
|
||||
bindingset[this]
|
||||
RequiresEncodingConfiguration() { any() }
|
||||
|
||||
/** Gets a textual representation of this kind of encoding requirement. */
|
||||
abstract string getKind();
|
||||
|
||||
/** Holds if `e` is an expression whose value must be encoded. */
|
||||
abstract predicate requiresEncoding(Node n);
|
||||
|
||||
/** Holds if `e` is a possible valid encoded value. */
|
||||
predicate isPossibleEncodedValue(Expr e) { none() }
|
||||
|
||||
/**
|
||||
* Holds if `encodedValue` is a possibly ill-encoded value that reaches
|
||||
* `sink`, where `sink` is an expression of kind `kind` that is required
|
||||
* to be encoded.
|
||||
*/
|
||||
predicate hasWrongEncoding(PathNode encodedValue, PathNode sink, string kind) {
|
||||
this.hasFlowPath(encodedValue, sink) and
|
||||
kind = this.getKind()
|
||||
}
|
||||
|
||||
override predicate isSource(Node source) {
|
||||
module RequiresEncodingConfig<EncodingConfigSig EncodingConfig> implements DataFlow::ConfigSig {
|
||||
predicate isSource(DataFlow::Node source) {
|
||||
// all encoded values that do not match this configuration are
|
||||
// considered sources
|
||||
exists(Expr e | e = source.asExpr() |
|
||||
e instanceof EncodedValue and
|
||||
not this.isPossibleEncodedValue(e)
|
||||
not EncodingConfig::isPossibleEncodedValue(e)
|
||||
)
|
||||
}
|
||||
|
||||
override predicate isSink(Node sink) { this.requiresEncoding(sink) }
|
||||
predicate isSink(DataFlow::Node sink) { EncodingConfig::requiresEncoding(sink) }
|
||||
|
||||
override predicate isSanitizer(Node sanitizer) { this.isPossibleEncodedValue(sanitizer.asExpr()) }
|
||||
predicate isBarrier(DataFlow::Node sanitizer) {
|
||||
EncodingConfig::isPossibleEncodedValue(sanitizer.asExpr())
|
||||
}
|
||||
|
||||
override int fieldFlowBranchLimit() { result = 0 }
|
||||
int fieldFlowBranchLimit() { result = 0 }
|
||||
}
|
||||
|
||||
/** An encoded value, for example a call to `HttpServerUtility.HtmlEncode`. */
|
||||
class EncodedValue extends Expr {
|
||||
EncodedValue() {
|
||||
any(RequiresEncodingConfiguration c).isPossibleEncodedValue(this)
|
||||
EncodingConfigurations::SqlExprEncodingConfig::isPossibleEncodedValue(this)
|
||||
or
|
||||
EncodingConfigurations::HtmlExprEncodingConfig::isPossibleEncodedValue(this)
|
||||
or
|
||||
EncodingConfigurations::UrlExprEncodingConfig::isPossibleEncodedValue(this)
|
||||
or
|
||||
this = any(SystemWebHttpUtility c).getAJavaScriptStringEncodeMethod().getACall()
|
||||
or
|
||||
@@ -86,18 +76,20 @@ class EncodedValue extends Expr {
|
||||
}
|
||||
|
||||
module EncodingConfigurations {
|
||||
module SqlExprEncodingConfig implements EncodingConfigSig {
|
||||
predicate requiresEncoding(DataFlow::Node n) { n instanceof SqlInjection::Sink }
|
||||
|
||||
predicate isPossibleEncodedValue(Expr e) { none() }
|
||||
}
|
||||
|
||||
/** An encoding configuration for SQL expressions. */
|
||||
class SqlExpr extends RequiresEncodingConfiguration {
|
||||
SqlExpr() { this = "SqlExpr" }
|
||||
|
||||
override string getKind() { result = "SQL expression" }
|
||||
|
||||
override predicate requiresEncoding(Node n) { n instanceof SqlInjection::Sink }
|
||||
module SqlExprConfig implements DataFlow::ConfigSig {
|
||||
import RequiresEncodingConfig<SqlExprEncodingConfig> as Super
|
||||
|
||||
// no override for `isPossibleEncodedValue` as SQL parameters should
|
||||
// be used instead of explicit encoding
|
||||
override predicate isSource(Node source) {
|
||||
super.isSource(source)
|
||||
predicate isSource(DataFlow::Node source) {
|
||||
Super::isSource(source)
|
||||
or
|
||||
// consider quote-replacing calls as additional sources for
|
||||
// SQL expressions (e.g., `s.Replace("\"", "\"\"")`)
|
||||
@@ -107,32 +99,62 @@ module EncodingConfigurations {
|
||||
mc.getArgument(0).getValue().regexpMatch("\"|'|`")
|
||||
)
|
||||
}
|
||||
|
||||
predicate isSink = Super::isSink/1;
|
||||
|
||||
predicate isBarrier = Super::isBarrier/1;
|
||||
|
||||
int fieldFlowBranchLimit() { result = Super::fieldFlowBranchLimit() }
|
||||
}
|
||||
|
||||
module SqlExpr = TaintTracking::Global<SqlExprConfig>;
|
||||
|
||||
module HtmlExprEncodingConfig implements EncodingConfigSig {
|
||||
predicate requiresEncoding(DataFlow::Node n) { n instanceof HtmlSink }
|
||||
|
||||
predicate isPossibleEncodedValue(Expr e) { e instanceof HtmlSanitizedExpr }
|
||||
}
|
||||
|
||||
/** An encoding configuration for HTML expressions. */
|
||||
class HtmlExpr extends RequiresEncodingConfiguration {
|
||||
HtmlExpr() { this = "HtmlExpr" }
|
||||
module HtmlExprConfig = RequiresEncodingConfig<HtmlExprEncodingConfig>;
|
||||
|
||||
override string getKind() { result = "HTML expression" }
|
||||
module HtmlExpr = TaintTracking::Global<HtmlExprConfig>;
|
||||
|
||||
override predicate requiresEncoding(Node n) { n instanceof HtmlSink }
|
||||
module UrlExprEncodingConfig implements EncodingConfigSig {
|
||||
predicate requiresEncoding(DataFlow::Node n) { n instanceof UrlRedirect::Sink }
|
||||
|
||||
override predicate isPossibleEncodedValue(Expr e) { e instanceof HtmlSanitizedExpr }
|
||||
predicate isPossibleEncodedValue(Expr e) { e instanceof UrlSanitizedExpr }
|
||||
}
|
||||
|
||||
/** An encoding configuration for URL expressions. */
|
||||
class UrlExpr extends RequiresEncodingConfiguration {
|
||||
UrlExpr() { this = "UrlExpr" }
|
||||
module UrlExprConfig = RequiresEncodingConfig<UrlExprEncodingConfig>;
|
||||
|
||||
override string getKind() { result = "URL expression" }
|
||||
module UrlExpr = TaintTracking::Global<UrlExprConfig>;
|
||||
|
||||
override predicate requiresEncoding(Node n) { n instanceof UrlRedirect::Sink }
|
||||
module Flow =
|
||||
DataFlow::MergePathGraph3<SqlExpr::PathNode, HtmlExpr::PathNode, UrlExpr::PathNode,
|
||||
SqlExpr::PathGraph, HtmlExpr::PathGraph, UrlExpr::PathGraph>;
|
||||
|
||||
override predicate isPossibleEncodedValue(Expr e) { e instanceof UrlSanitizedExpr }
|
||||
/**
|
||||
* Holds if `encodedValue` is a possibly ill-encoded value that reaches
|
||||
* `sink`, where `sink` is an expression of kind `kind` that is required
|
||||
* to be encoded.
|
||||
*/
|
||||
predicate hasWrongEncoding(Flow::PathNode encodedValue, Flow::PathNode sink, string kind) {
|
||||
SqlExpr::flowPath(encodedValue.asPathNode1(), sink.asPathNode1()) and
|
||||
kind = "SQL expression"
|
||||
or
|
||||
HtmlExpr::flowPath(encodedValue.asPathNode2(), sink.asPathNode2()) and
|
||||
kind = "HTML expression"
|
||||
or
|
||||
UrlExpr::flowPath(encodedValue.asPathNode3(), sink.asPathNode3()) and
|
||||
kind = "URL expression"
|
||||
}
|
||||
}
|
||||
|
||||
from RequiresEncodingConfiguration c, PathNode encodedValue, PathNode sink, string kind
|
||||
where c.hasWrongEncoding(encodedValue, sink, kind)
|
||||
from
|
||||
EncodingConfigurations::Flow::PathNode encodedValue, EncodingConfigurations::Flow::PathNode sink,
|
||||
string kind
|
||||
where EncodingConfigurations::hasWrongEncoding(encodedValue, sink, kind)
|
||||
select sink.getNode(), encodedValue, sink, "This " + kind + " may include data from a $@.",
|
||||
encodedValue.getNode(), "possibly inappropriately encoded value"
|
||||
|
||||
Reference in New Issue
Block a user