mirror of
https://github.com/github/codeql.git
synced 2026-04-30 19:26:02 +02:00
C#: Add queries to check untrusted data flow to external APIs
This commit is contained in:
Tamas Vajk
@@ -0,0 +1,14 @@
|
||||
using System;
|
||||
using System.Text;
|
||||
using System.Web;
|
||||
|
||||
public class UntrustedData : IHttpHandler
|
||||
{
|
||||
public void ProcessRequest(HttpContext ctx)
|
||||
{
|
||||
var name = ctx.Request.QueryString["name"];
|
||||
ctx.Response.Write(name);
|
||||
}
|
||||
|
||||
public bool IsReusable => true;
|
||||
}
|
||||
@@ -0,0 +1,44 @@
|
||||
<!DOCTYPE qhelp PUBLIC
|
||||
"-//Semmle//qhelp//EN"
|
||||
"qhelp.dtd">
|
||||
<qhelp>
|
||||
<overview>
|
||||
<p>Using unsanitized untrusted data in an external API can cause a variety of security issues. This query reports
|
||||
all external APIs that are used with untrusted data, along with how frequently the API is used, and how many
|
||||
unique sources of untrusted data flow to this API. This query is designed primarily to help identify which APIs
|
||||
may be relevant for security analysis of this application.</p>
|
||||
|
||||
<p>An external API is defined as a call to a callable (method/property accessor/operator/...) that is not defined
|
||||
in the source code, not overridden in the source code, and is not modeled as a taint step in the default taint library.
|
||||
External APIs may be from the .NET standard library, third-party dependencies or from internal dependencies. The query
|
||||
will report the signature with a fully qualified name, along with either <code>[param x]</code>, where <code>x</code>
|
||||
indicates the position of the parameter receiving the untrusted data or <code>[qualifier]</code> indicating the untrusted
|
||||
data is used as the qualifier to the call.</p>
|
||||
|
||||
</overview>
|
||||
<recommendation>
|
||||
|
||||
<p>For each result:</p>
|
||||
|
||||
<ul>
|
||||
<li>If the result highlights a known sink, no action is required.</li>
|
||||
<li>If the result highlights an unknown sink for a problem, then add modeling for the sink to the relevant query.</li>
|
||||
<li>If the result represents a call to an external API which transfers taint, add the appropriate modeling, and
|
||||
re-run the query to determine what new results have appeared due to this additional modeling.</li>
|
||||
</ul>
|
||||
|
||||
<p>Otherwise, the result is likely uninteresting. Custom versions of this query can extend the <code>SafeExternalAPICallable</code>
|
||||
class to exclude known safe external APIs from future analysis.</p>
|
||||
|
||||
</recommendation>
|
||||
<example>
|
||||
|
||||
<p>If the query were to return the API <code>System.Web.HttpResponse.Write(string) [param 0]</code>
|
||||
then we should first consider whether this a security relevant sink. In this case, this is writing to a HTTP response, so we should
|
||||
consider whether this is an XSS sink. If it is, we should confirm that it is handled by the XSS query.</p>
|
||||
|
||||
</example>
|
||||
<references>
|
||||
|
||||
</references>
|
||||
</qhelp>
|
||||
@@ -0,0 +1,18 @@
|
||||
/**
|
||||
* @name Frequency counts for external APIs that are used with untrusted data
|
||||
* @description This reports the external APIs that are used with untrusted data, along with how
|
||||
* frequently the API is called, and how many unique sources of untrusted data flow
|
||||
* to it.
|
||||
* @id csharp/count-untrusted-data-external-api
|
||||
* @kind table
|
||||
* @tags security external/cwe/cwe-20
|
||||
*/
|
||||
|
||||
import csharp
|
||||
import semmle.code.csharp.security.dataflow.ExternalAPIs
|
||||
import semmle.code.csharp.dataflow.DataFlow
|
||||
|
||||
from ExternalAPIUsedWithUntrustedData externalAPI
|
||||
select externalAPI, count(externalAPI.getUntrustedDataNode()) as numberOfUses,
|
||||
externalAPI.getNumberOfUntrustedSources() as numberOfUntrustedSources order by
|
||||
numberOfUntrustedSources desc
|
||||
@@ -0,0 +1,49 @@
|
||||
<!DOCTYPE qhelp PUBLIC
|
||||
"-//Semmle//qhelp//EN"
|
||||
"qhelp.dtd">
|
||||
<qhelp>
|
||||
<overview>
|
||||
<p>Using unsanitized untrusted data in an external API can cause a variety of security issues. This query reports
|
||||
external APIs that use untrusted data. The results are not filtered so that you can audit all examples. The query
|
||||
provides data for security reviews of the application and you can also use it to identify external APIs that should
|
||||
be modeled as either taint steps, or sinks for specific problems.</p>
|
||||
|
||||
<p>An external API is defined as a call to a callable (method/property accessor/operator/...) that is not defined
|
||||
in the source code, not overridden in the source code, and is not modeled as a taint step in the default taint library.
|
||||
External APIs may be from the .NET standard library, third-party dependencies or from internal dependencies. The query
|
||||
reports uses of untrusted data in either the qualifier or as one of the arguments of external APIs.</p>
|
||||
|
||||
</overview>
|
||||
<recommendation>
|
||||
|
||||
<p>For each result:</p>
|
||||
|
||||
<ul>
|
||||
<li>If the result highlights a known sink, confirm that the result is reported by the relevant query, or
|
||||
that the result is a false positive because this data is sanitized.</li>
|
||||
<li>If the result highlights an unknown sink for a problem, then add modeling for the sink to the relevant query,
|
||||
and confirm that the result is either found, or is safe due to appropriate sanitization.</li>
|
||||
<li>If the result represents a call to an external API that transfers taint, add the appropriate modeling, and
|
||||
re-run the query to determine what new results have appeared due to this additional modeling.</li>
|
||||
</ul>
|
||||
|
||||
<p>Otherwise, the result is likely uninteresting. Custom versions of this query can extend the <code>SafeExternalAPICallable</code>
|
||||
class to exclude known safe external APIs from future analysis.</p>
|
||||
|
||||
</recommendation>
|
||||
<example>
|
||||
|
||||
<p>A query string parameter is read from <code>HttpRequest</code> and then ultimately used in a call to the
|
||||
<code>HttpResponse.Write</code> external API:</p>
|
||||
|
||||
<sample src="ExternalAPISinkExample.cs" />
|
||||
|
||||
<p>This is an XSS sink. The XSS query should therefore be reviewed to confirm that this sink is appropriately modeled,
|
||||
and if it is, to confirm that the query reports this particular result, or that the result is a false positive due to
|
||||
some existing sanitization.</p>
|
||||
|
||||
</example>
|
||||
<references>
|
||||
|
||||
</references>
|
||||
</qhelp>
|
||||
@@ -0,0 +1,20 @@
|
||||
/**
|
||||
* @name Untrusted data passed to external API
|
||||
* @description Data provided remotely is used in this external API without sanitization, which could be a security risk.
|
||||
* @id csharp/untrusted-data-to-external-api
|
||||
* @kind path-problem
|
||||
* @precision low
|
||||
* @problem.severity error
|
||||
* @tags security external/cwe/cwe-20
|
||||
*/
|
||||
|
||||
import csharp
|
||||
import semmle.code.csharp.dataflow.TaintTracking
|
||||
import semmle.code.csharp.security.dataflow.ExternalAPIs
|
||||
import DataFlow::PathGraph
|
||||
|
||||
from UntrustedDataToExternalAPIConfig config, DataFlow::PathNode source, DataFlow::PathNode sink
|
||||
where config.hasFlowPath(source, sink)
|
||||
select sink, source, sink,
|
||||
"Call to " + sink.getNode().(ExternalAPIDataNode).getCallableDescription() +
|
||||
" with untrusted data from $@.", source, source.toString()
|
||||
@@ -444,6 +444,15 @@ class SystemStringClass extends StringType {
|
||||
result.getNumberOfParameters() = 1 and
|
||||
result.getReturnType() instanceof BoolType
|
||||
}
|
||||
|
||||
/** Gets the `IsNullOrWhiteSpace(string)` method. */
|
||||
Method getIsNullOrWhiteSpaceMethod() {
|
||||
result.getDeclaringType() = this and
|
||||
result.hasName("IsNullOrWhiteSpace") and
|
||||
result.isStatic() and
|
||||
result.getNumberOfParameters() = 1 and
|
||||
result.getReturnType() instanceof BoolType
|
||||
}
|
||||
}
|
||||
|
||||
/** A `ToString()` method. */
|
||||
|
||||
@@ -0,0 +1,122 @@
|
||||
/**
|
||||
* Definitions for reasoning about untrusted data used in APIs defined outside the
|
||||
* database.
|
||||
*/
|
||||
|
||||
import csharp
|
||||
import semmle.code.csharp.dataflow.flowsources.Remote
|
||||
import semmle.code.csharp.dataflow.TaintTracking
|
||||
import semmle.code.csharp.frameworks.System
|
||||
|
||||
/**
|
||||
* A `Callable` that is considered a "safe" external API from a security perspective.
|
||||
*/
|
||||
abstract class SafeExternalAPICallable extends Callable { }
|
||||
|
||||
/** The default set of "safe" external APIs. */
|
||||
private class DefaultSafeExternalAPICallable extends SafeExternalAPICallable {
|
||||
DefaultSafeExternalAPICallable() {
|
||||
this instanceof EqualsMethod or
|
||||
this instanceof IEquatableEqualsMethod or
|
||||
this = any(SystemObjectClass s).getEqualsMethod() or
|
||||
this = any(SystemObjectClass s).getReferenceEqualsMethod() or
|
||||
this = any(SystemObjectClass s).getStaticEqualsMethod() or
|
||||
this = any(SystemObjectClass s).getGetTypeMethod() or
|
||||
this = any(SystemStringClass s).getEqualsMethod() or
|
||||
this = any(SystemStringClass s).getEqualsOperator() or
|
||||
this = any(SystemStringClass s).getIsNullOrEmptyMethod() or
|
||||
this = any(SystemStringClass s).getIsNullOrWhiteSpaceMethod() or
|
||||
this.getName().regexpMatch("Count|get_Count|get_Length")
|
||||
}
|
||||
}
|
||||
|
||||
/** A node representing data being passed to an external API. */
|
||||
class ExternalAPIDataNode extends DataFlow::Node {
|
||||
Call call;
|
||||
int i;
|
||||
|
||||
ExternalAPIDataNode() {
|
||||
(
|
||||
// Argument to call
|
||||
this.asExpr() = call.getArgument(i)
|
||||
or
|
||||
// Qualifier to call
|
||||
this.asExpr() = call.getChild(i) and
|
||||
i = -1 and
|
||||
// extension methods are covered above
|
||||
not call.getTarget().(Method).isExtensionMethod()
|
||||
) and
|
||||
// Defined outside the source archive
|
||||
not call.getTarget().fromSource() and
|
||||
// Not a call to a method which is overridden in source
|
||||
not exists(Virtualizable m |
|
||||
m.overridesOrImplementsOrEquals(call.getTarget().getSourceDeclaration()) and
|
||||
m.fromSource()
|
||||
) and
|
||||
// Not already modeled as a taint step
|
||||
not exists(DataFlow::Node next | TaintTracking::localTaintStep(this, next)) and
|
||||
// Not a call to a known safe external API
|
||||
not call.getTarget().getSourceDeclaration() instanceof SafeExternalAPICallable
|
||||
}
|
||||
|
||||
/** Gets the called API `Callable`. */
|
||||
Callable getCallable() { result = call.getTarget().getSourceDeclaration() }
|
||||
|
||||
/** Gets the index which is passed untrusted data (where -1 indicates the qualifier). */
|
||||
int getIndex() { result = i }
|
||||
|
||||
/** Gets the description of the callable being called. */
|
||||
string getCallableDescription() { result = getCallable().getQualifiedName() }
|
||||
}
|
||||
|
||||
/** A configuration for tracking flow from `RemoteFlowSource`s to `ExternalAPIDataNode`s. */
|
||||
class UntrustedDataToExternalAPIConfig extends TaintTracking::Configuration {
|
||||
UntrustedDataToExternalAPIConfig() { this = "UntrustedDataToExternalAPIConfig" }
|
||||
|
||||
override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
|
||||
|
||||
override predicate isSink(DataFlow::Node sink) { sink instanceof ExternalAPIDataNode }
|
||||
}
|
||||
|
||||
/** A node representing untrusted data being passed to an external API. */
|
||||
class UntrustedExternalAPIDataNode extends ExternalAPIDataNode {
|
||||
UntrustedExternalAPIDataNode() { any(UntrustedDataToExternalAPIConfig c).hasFlow(_, this) }
|
||||
|
||||
/** Gets a source of untrusted data which is passed to this external API data node. */
|
||||
DataFlow::Node getAnUntrustedSource() {
|
||||
any(UntrustedDataToExternalAPIConfig c).hasFlow(result, this)
|
||||
}
|
||||
}
|
||||
|
||||
private newtype TExternalAPI =
|
||||
TExternalAPIParameter(Callable m, int index) {
|
||||
exists(UntrustedExternalAPIDataNode n |
|
||||
m = n.getCallable().getSourceDeclaration() and
|
||||
index = n.getIndex()
|
||||
)
|
||||
}
|
||||
|
||||
/** An external API which is used with untrusted data. */
|
||||
class ExternalAPIUsedWithUntrustedData extends TExternalAPI {
|
||||
/** Gets a possibly untrusted use of this external API. */
|
||||
UntrustedExternalAPIDataNode getUntrustedDataNode() {
|
||||
this = TExternalAPIParameter(result.getCallable().getSourceDeclaration(), result.getIndex())
|
||||
}
|
||||
|
||||
/** Gets the number of untrusted sources used with this external API. */
|
||||
int getNumberOfUntrustedSources() {
|
||||
result = count(getUntrustedDataNode().getAnUntrustedSource())
|
||||
}
|
||||
|
||||
/** Gets a textual representation of this element. */
|
||||
string toString() {
|
||||
exists(Callable m, int index, string indexString |
|
||||
if index = -1 then indexString = "qualifier" else indexString = "param " + index
|
||||
|
|
||||
this = TExternalAPIParameter(m, index) and
|
||||
result =
|
||||
m.getDeclaringType().getQualifiedName() + "." + m.toStringWithTypes() + " [" + indexString +
|
||||
"]"
|
||||
)
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,2 @@
|
||||
| System.Web.HttpRequest.get_QueryString() [qualifier] | 1 | 1 |
|
||||
| System.Web.HttpResponse.Write(string) [param 0] | 1 | 1 |
|
||||
@@ -0,0 +1 @@
|
||||
Security Features/CWE-020/ExternalAPIsUsedWithUntrustedData.ql
|
||||
@@ -0,0 +1,16 @@
|
||||
// semmle-extractor-options: /r:${testdir}/../../../resources/assemblies/System.Web.dll /r:${testdir}/../../../resources/assemblies/System.Web.ApplicationServices.dll /r:${testdir}/../../../resources/assemblies/System.Data.dll /r:System.Text.RegularExpressions.dll /r:System.Collections.Specialized.dll /r:System.Data.Common.dll /r:System.Security.Cryptography.X509Certificates.dll /r:System.Runtime.InteropServices.dll
|
||||
|
||||
using System;
|
||||
using System.Text;
|
||||
using System.Web;
|
||||
|
||||
public class UntrustedData : IHttpHandler
|
||||
{
|
||||
public void ProcessRequest(HttpContext ctx)
|
||||
{
|
||||
var name = ctx.Request.QueryString["name"];
|
||||
ctx.Response.Write(name);
|
||||
}
|
||||
|
||||
public bool IsReusable => true;
|
||||
}
|
||||
@@ -0,0 +1,9 @@
|
||||
edges
|
||||
| UntrustedData.cs:11:20:11:42 | access to property QueryString : NameValueCollection | UntrustedData.cs:12:28:12:31 | access to local variable name |
|
||||
nodes
|
||||
| UntrustedData.cs:11:20:11:30 | access to property Request | semmle.label | access to property Request |
|
||||
| UntrustedData.cs:11:20:11:42 | access to property QueryString : NameValueCollection | semmle.label | access to property QueryString : NameValueCollection |
|
||||
| UntrustedData.cs:12:28:12:31 | access to local variable name | semmle.label | access to local variable name |
|
||||
#select
|
||||
| UntrustedData.cs:11:20:11:30 | access to property Request | UntrustedData.cs:11:20:11:30 | access to property Request | UntrustedData.cs:11:20:11:30 | access to property Request | Call to System.Web.HttpRequest.get_QueryString with untrusted data from $@. | UntrustedData.cs:11:20:11:30 | access to property Request | access to property Request |
|
||||
| UntrustedData.cs:12:28:12:31 | access to local variable name | UntrustedData.cs:11:20:11:42 | access to property QueryString : NameValueCollection | UntrustedData.cs:12:28:12:31 | access to local variable name | Call to System.Web.HttpResponse.Write with untrusted data from $@. | UntrustedData.cs:11:20:11:42 | access to property QueryString : NameValueCollection | access to property QueryString : NameValueCollection |
|
||||
@@ -0,0 +1 @@
|
||||
Security Features/CWE-020/UntrustedDataToExternalAPI.ql
|
||||
Reference in New Issue
Block a user