Skip to content

Commit

Permalink
C#: Turn the insecure certificate validation query into a data flow q…
Browse files Browse the repository at this point in the history
…uery.
  • Loading branch information
michaelnebel committed Sep 27, 2024
1 parent e67e8a9 commit c1f6a55
Show file tree
Hide file tree
Showing 2 changed files with 162 additions and 88 deletions.
98 changes: 10 additions & 88 deletions csharp/ql/src/experimental/CWE-295/InsecureCertificateValidation.ql
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/**
* @name Unsafe `CertificateValidationCallback` use.
* @description Using a `CertificateValidationCallback` that always returns `true` is insecure, as it allows any certificate to be accepted as valid.
* @kind alert
* @kind path-problem
* @problem.severity error
* @precision high
* @id cs/unsafe-certificate-validation
Expand All @@ -11,90 +11,12 @@
*/

import csharp

/**
* Provides an abstract base class for properties related to certificate validation.
*/
abstract private class CertificateValidationProperty extends Property { }

/**
* Represents the `ServerCertificateValidationCallback` property of the `ServicePointManager` class.
*/
private class ServicePointManagerServerCertificateValidationCallback extends CertificateValidationProperty
{
ServicePointManagerServerCertificateValidationCallback() {
this.getDeclaringType().hasFullyQualifiedName("System.Net", "ServicePointManager") and
this.hasName("ServerCertificateValidationCallback")
}
}

/**
* Represents the `ServerCertificateCustomValidationCallback` property of the `HttpClientHandler` class.
*/
private class HttpClientHandlerServerCertificateCustomValidationCallback extends CertificateValidationProperty
{
HttpClientHandlerServerCertificateCustomValidationCallback() {
this.getDeclaringType().hasFullyQualifiedName("System.Net.Http", "HttpClientHandler") and
this.hasName("ServerCertificateCustomValidationCallback")
}
}

/**
* Represents the creation of an `SslStream` object.
*/
private class SslStreamCreation extends ObjectCreation {
SslStreamCreation() {
this.getObjectType().hasFullyQualifiedName("System.Net.Security", "SslStream")
}

/**
* Gets the expression used as the server certificate validation callback argument
* in the creation of the `SslStream` object.
*/
Expr getServerCertificateValidationCallback() { result = this.getArgument(2) }
}

/**
* Represents the `ServerCertificateValidationCallback` property of the `HttpWebRequest` class.
*/
private class HttpWebRequestServerCertificateValidationCallback extends CertificateValidationProperty
{
HttpWebRequestServerCertificateValidationCallback() {
this.getDeclaringType().hasFullyQualifiedName("System.Net", "HttpWebRequest") and
this.hasName("ServerCertificateValidationCallback")
}
}

/**
* Holds if `c` always returns `true`.
*/
private predicate alwaysReturnsTrue(Callable c) {
forex(ReturnStmt rs | rs.getEnclosingCallable() = c |
rs.getExpr().(BoolLiteral).getBoolValue() = true
)
or
c.getBody().(BoolLiteral).getBoolValue() = true
}

/**
* Gets the actual callable object referred to by expression `e`.
* This can be a direct reference to a callable, a delegate creation, or an anonymous function.
*/
Callable getActualCallable(Expr e) {
exists(Expr dcArg | dcArg = e.(DelegateCreation).getArgument() |
result = dcArg.(CallableAccess).getTarget() or result = dcArg.(AnonymousFunctionExpr)
)
or
result = e.(Callable)
}

from Expr e, Callable c
where
[
any(SslStreamCreation yy).getServerCertificateValidationCallback(),
any(CertificateValidationProperty xx).getAnAssignedValue()
] = e and
getActualCallable(e) = c and
alwaysReturnsTrue(c)
select e, "$@ that is defined $@ and accepts any certificate as valid, is used here.", e,
"This certificate callback", c, "here"
import DataFlow
import InsecureCertificateValidationQuery
import InsecureCertificateValidation::PathGraph

from InsecureCertificateValidation::PathNode source, InsecureCertificateValidation::PathNode sink
where InsecureCertificateValidation::flowPath(source, sink)
select sink.getNode(), source, sink,
"$@ that is defined $@ and accepts any certificate as valid, is used here.", sink,
"This certificate callback", source, "here"
Original file line number Diff line number Diff line change
@@ -0,0 +1,152 @@
/**
* Provides a taint-tracking configuration for reasoning
* about insecure certificate validation vulnerabilities.
*/

private import csharp
private import DataFlow
private import semmle.code.csharp.controlflow.Guards

/**
* A source specific to unsafe certificate validation vulnerabilities.
*/
abstract private class Source extends DataFlow::Node { }

/**
* A sink specific to unsafe certificate validation vulnerabilities.
*/
abstract private class Sink extends DataFlow::Node { }

/**
* A sanitizer specific to unsafe certificate validation vulnerabilities.
*/
abstract private class Sanitizer extends DataFlow::ExprNode { }

/**
* Provides an abstract base class for properties related to certificate validation.
*/
abstract private class CertificateValidationProperty extends Property { }

/**
* Represents the `ServerCertificateValidationCallback` property of the `ServicePointManager` class.
*/
private class ServicePointManagerServerCertificateValidationCallback extends CertificateValidationProperty
{
ServicePointManagerServerCertificateValidationCallback() {
this.getDeclaringType().hasFullyQualifiedName("System.Net", "ServicePointManager") and
this.hasName("ServerCertificateValidationCallback")
}
}

/**
* Represents the `ServerCertificateCustomValidationCallback` property of the `HttpClientHandler` class.
*/
private class HttpClientHandlerServerCertificateCustomValidationCallback extends CertificateValidationProperty
{
HttpClientHandlerServerCertificateCustomValidationCallback() {
this.getDeclaringType().hasFullyQualifiedName("System.Net.Http", "HttpClientHandler") and
this.hasName("ServerCertificateCustomValidationCallback")
}
}

/**
* Represents the `ServerCertificateValidationCallback` property of the `HttpWebRequest` class.
*/
private class HttpWebRequestServerCertificateValidationCallback extends CertificateValidationProperty
{
HttpWebRequestServerCertificateValidationCallback() {
this.getDeclaringType().hasFullyQualifiedName("System.Net", "HttpWebRequest") and
this.hasName("ServerCertificateValidationCallback")
}
}

/**
* Represents the creation of an `SslStream` object.
*/
private class SslStreamCreation extends ObjectCreation {
SslStreamCreation() {
this.getObjectType().hasFullyQualifiedName("System.Net.Security", "SslStream")
}

/**
* Gets the expression used as the server certificate validation callback argument
* in the creation of the `SslStream` object.
*/
Expr getServerCertificateValidationCallback() { result = this.getArgument(2) }
}

/**
* Holds if `c` always returns `true`.
*/
private predicate alwaysReturnsTrue(Callable c) {
forex(ReturnStmt rs | rs.getEnclosingCallable() = c |
rs.getExpr().(BoolLiteral).getBoolValue() = true
)
or
c.getBody().(BoolLiteral).getBoolValue() = true
}

/**
* Gets the actual callable object referred to by expression `e`.
* This can be a direct reference to a callable, a delegate creation, or an anonymous function.
*/
private Callable getActualCallable(Expr e) {
exists(Expr dcArg | dcArg = e.(DelegateCreation).getArgument() |
result = dcArg.(CallableAccess).getTarget() or
result = dcArg.(AnonymousFunctionExpr)
)
or
result = e
}

private predicate ignoreCertificateValidation(Guard guard, AbstractValue v) {
guard =
any(PropertyAccess access |
access.getProperty().hasFullyQualifiedName("", "Settings", "IgnoreCertificateValidation") and
v.(AbstractValues::BooleanValue).getValue() = true
)
}

private class AddIgnoreCheck extends Sanitizer {
AddIgnoreCheck() {
exists(Guard g, AbstractValues::BooleanValue v | ignoreCertificateValidation(g, v) |
g.controlsNode(this.getControlFlowNode(), v)
)
}
}

private class CallableAlwaysReturnsTrue extends Callable {
CallableAlwaysReturnsTrue() { alwaysReturnsTrue(this) }
}

private class AddCallableAlwaysReturnsTrue extends Source {
AddCallableAlwaysReturnsTrue() {
getActualCallable(this.asExpr()) instanceof CallableAlwaysReturnsTrue
}
}

private class AddSinks extends Sink {
AddSinks() {
exists(Expr e | e = this.asExpr() |
e =
[
any(CertificateValidationProperty p).getAnAssignedValue(),
any(SslStreamCreation yy).getServerCertificateValidationCallback()
] and
not e.getFile().getAbsolutePath().matches("example")
)
}
}

private module InsecureCertificateValidationConfig implements DataFlow::ConfigSig {
predicate isSource(DataFlow::Node node) { node instanceof Source }

predicate isBarrier(DataFlow::Node node) { node instanceof Sanitizer }

predicate isSink(DataFlow::Node node) { node instanceof Sink }
}

/**
* A taint-tracking module for insecure certificate validation vulnerabilities.
*/
module InsecureCertificateValidation = TaintTracking::Global<InsecureCertificateValidationConfig>;

0 comments on commit c1f6a55

Please sign in to comment.