Merge pull request #1053 from hvitved/csharp/dispatch-tweak

C#: `Dispatch.qll` performance tweaks
This commit is contained in:
Calum Grant
2019-03-07 16:21:29 +00:00
committed by GitHub
13 changed files with 78 additions and 63 deletions

View File

@@ -57,7 +57,7 @@ namespace Semmle.Util
/// Finds the path for the program <paramref name="prog"/> based on the
/// <code>PATH</code> environment variable, and in the case of Windows the
/// <code>PATHEXT</code> environment variable.
///
///
/// Returns <code>null</code> of no path can be found.
/// </summary>
public static string FindProgramOnPath(string prog)

View File

@@ -7,7 +7,7 @@ public class HttpHandler : IHttpHandler
public void ProcessRequest(HttpContext ctx)
{
string format = ctx.Request.QueryString["nameformat"];
// BAD: Uncontrolled format string.
FormattedName = string.Format(format, Surname, Forenames);
}

View File

@@ -222,11 +222,43 @@ private module Internal {
}
}
private class FieldOrProperty extends Assignable {
FieldOrProperty() {
this instanceof Field or
this instanceof Property
private class DynamicFieldOrProperty extends Assignable {
DynamicFieldOrProperty() {
(
this instanceof Field or
this instanceof Property
) and
this.getName() = any(DynamicMemberAccess dma).getLateBoundTargetName()
}
predicate isMemberOf(string name, ValueOrRefType t) {
name = this.getName() and t.hasMember(this)
}
}
private class TypeWithDynamicFieldOrProperty extends ValueOrRefType {
DynamicFieldOrProperty fp;
TypeWithDynamicFieldOrProperty() { fp.isMemberOf(_, this) }
predicate isImplicitlyConvertibleTo(string name, Type t) {
name = fp.getName() and
this.isImplicitlyConvertibleTo(t)
}
}
pragma[noinline]
private predicate isPossibleDynamicMemberAccessQualifierType(
DynamicMemberAccess dma, string name, TypeWithDynamicFieldOrProperty t
) {
exists(Type qt, boolean isExact |
qt = getAPossibleType(dma.getQualifier(), isExact) and
name = dma.getLateBoundTargetName()
|
isExact = true and t = qt
or
isExact = false and t.isImplicitlyConvertibleTo(name, qt)
)
}
/**
@@ -235,14 +267,10 @@ private module Internal {
* corresponding to the type of a relevant field or property are included.
*/
private Type getAPossibleType(Expr e, boolean isExact) {
exists(ValueOrRefType qualifierType, FieldOrProperty fp, boolean qualifierTypeIsExact |
qualifierType = getAPossibleTypeDynamicMemberAccessQualifier(e, qualifierTypeIsExact, fp)
exists(DynamicFieldOrProperty fp, string name, TypeWithDynamicFieldOrProperty t |
isPossibleDynamicMemberAccessQualifierType(e, name, t) and
fp.isMemberOf(name, t)
|
(
if qualifierTypeIsExact = true
then qualifierType.hasMember(fp)
else fp.getDeclaringType().isImplicitlyConvertibleTo(qualifierType)
) and
result = fp.getType() and
isExact = false
)
@@ -251,13 +279,6 @@ private module Internal {
result = getASourceType(e, isExact)
}
private Type getAPossibleTypeDynamicMemberAccessQualifier(
DynamicMemberAccess dma, boolean isExact, FieldOrProperty fp
) {
result = getAPossibleType(dma.getQualifier(), isExact) and
fp.getName() = dma.getLateBoundTargetName()
}
/**
* Provides the predicate `getASourceType()` for finding all relevant source
* types for a given expression.
@@ -799,22 +820,14 @@ private module Internal {
// conflicting types (for example, `Tuple<int, string>` is considered
// compatible with `Tuple<T, T>`).
override RuntimeCallable getADynamicTarget() {
// Condition 1
result = getADynamicTargetCandidate() and
// Condition 2
forall(int i | i in [0 .. getNumberOfArguments() - 1] |
result = getADynamicTargetCandidateWithCompatibleArg(i)
)
result = this.getADynamicTarget(this.getNumberOfArguments() - 1)
}
private RuntimeCallable getADynamicTargetCandidateWithCompatibleArg(int i) {
result = getADynamicTargetCandidateWithCompatibleArg1(i) or
result = getADynamicTargetCandidateWithCompatibleArg2(i)
}
pragma[noinline]
private RuntimeCallable getADynamicTargetCandidateWithCompatibleArg1(int i) {
result = this.getADynamicTargetCandidate() and
private RuntimeCallable getADynamicTarget(int i) {
i = -1 and
result = this.getADynamicTargetCandidate()
or
result = this.getADynamicTarget(i - 1) and
exists(Type parameterType, Type argumentType |
parameterType = this.getAParameterType(result, i) and
argumentType = getAPossibleType(this.getArgument(i), _)
@@ -827,6 +840,12 @@ private module Internal {
or
reflectionOrDynamicArgEqualsParamModuloTypeParameters(argumentType, parameterType)
)
or
result = this.getADynamicTarget(i - 1) and
exists(Type parameterType, Type t | parameterType = this.getAParameterType(result, i) |
this.argumentConvConstExpr(i, t) and
t.isImplicitlyConvertibleTo(parameterType)
)
}
private Type getAParameterType(RuntimeCallable c, int i) {
@@ -840,15 +859,6 @@ private module Internal {
)
}
pragma[noinline]
private RuntimeCallable getADynamicTargetCandidateWithCompatibleArg2(int i) {
result = this.getADynamicTargetCandidate() and
exists(Type parameterType, Type t | parameterType = this.getAParameterType(result, i) |
this.argumentConvConstExpr(i, t) and
t.isImplicitlyConvertibleTo(parameterType)
)
}
pragma[noinline]
private predicate argumentConvConstExpr(int i, Type t) {
convConstantExpr(this.getArgument(i), t)
@@ -954,7 +964,7 @@ private module Internal {
*/
private predicate isReflectionOrDynamicCallArgumentWithTypeParameters(Type argType, Type paramType) {
exists(DispatchReflectionOrDynamicCall call, Parameter p, int i, int j |
p = call.getAStaticTarget().getParameter(i) and
p = call.getADynamicTargetCandidate().getParameter(i) and
(
if p.isParams()
then (

View File

@@ -654,6 +654,14 @@ class QualifiableExpr extends Expr, @qualifiable_expr {
predicate isConditional() { conditional_access(this) }
}
private Expr getAnAssignOrForeachChild() {
result = any(AssignExpr e).getLValue()
or
result = any(ForeachStmt fs).getVariableDeclTuple()
or
result = getAnAssignOrForeachChild().getAChildExpr()
}
/**
* An expression representing a tuple, for example
* `(1, 2)` on line 2 or `(var x, var y)` on line 5 in
@@ -678,10 +686,7 @@ class TupleExpr extends Expr, @tuple_expr {
Expr getAnArgument() { result = getArgument(_) }
/** Holds if this tuple is a read access. */
predicate isReadAccess() {
not exists(AssignExpr e | this = e.getLValue().getAChildExpr*()) and
not exists(ForeachStmt fs | this = fs.getVariableDeclTuple().getAChildExpr*())
}
predicate isReadAccess() { not this = getAnAssignOrForeachChild() }
}
/**

View File

@@ -166,4 +166,4 @@ class SplittingStressTest
;
;
}
}
}

View File

@@ -44,7 +44,7 @@ class Program
{
if (obj2 == null)
{
obj2 = null;
obj2 = null;
}
}
}

View File

@@ -55,6 +55,6 @@ public class CommandInjectionHandler : IHttpHandler
void OnButtonClicked()
{
// BAD: Use the Roslyn APIs to dynamically evaluate C#
CSharpScript.EvaluateAsync(box1.Text);
CSharpScript.EvaluateAsync(box1.Text);
}
}

View File

@@ -5,8 +5,8 @@ nodes
| CodeInjection.cs:25:23:25:45 | access to property QueryString |
| CodeInjection.cs:31:64:31:67 | access to local variable code |
| CodeInjection.cs:42:36:42:39 | access to local variable code |
| CodeInjection.cs:58:33:58:41 | access to property Text |
| CodeInjection.cs:58:36:58:44 | access to property Text |
#select
| CodeInjection.cs:31:64:31:67 | access to local variable code | CodeInjection.cs:25:23:25:45 | access to property QueryString | CodeInjection.cs:31:64:31:67 | access to local variable code | $@ flows to here and is compiled as code. | CodeInjection.cs:25:23:25:45 | access to property QueryString | User-provided value |
| CodeInjection.cs:42:36:42:39 | access to local variable code | CodeInjection.cs:25:23:25:45 | access to property QueryString | CodeInjection.cs:42:36:42:39 | access to local variable code | $@ flows to here and is compiled as code. | CodeInjection.cs:25:23:25:45 | access to property QueryString | User-provided value |
| CodeInjection.cs:58:33:58:41 | access to property Text | CodeInjection.cs:58:33:58:41 | access to property Text | CodeInjection.cs:58:33:58:41 | access to property Text | $@ flows to here and is compiled as code. | CodeInjection.cs:58:33:58:41 | access to property Text | User-provided value |
| CodeInjection.cs:58:36:58:44 | access to property Text | CodeInjection.cs:58:36:58:44 | access to property Text | CodeInjection.cs:58:36:58:44 | access to property Text | $@ flows to here and is compiled as code. | CodeInjection.cs:58:36:58:44 | access to property Text | User-provided value |

View File

@@ -9,16 +9,16 @@ public class TaintedPathHandler : IHttpHandler
public void ProcessRequest(HttpContext ctx)
{
String path = ctx.Request.QueryString["page"];
// BAD: Uncontrolled format string.
String.Format(path, "Do not do this");
// BAD: Using an IFormatProvider.
String.Format((IFormatProvider)null, path, "Do not do this");
// GOOD: Not the format string.
String.Format("Do not do this", path);
// GOOD: Not the format string.
String.Format((IFormatProvider)null, "Do not do this", path);
}
@@ -27,7 +27,7 @@ public class TaintedPathHandler : IHttpHandler
void OnButtonClicked()
{
// BAD: Uncontrolled format string.
String.Format(box1.Text, "Do not do this");
// BAD: Uncontrolled format string.
String.Format(box1.Text, "Do not do this");
}
}

View File

@@ -8,11 +8,11 @@ nodes
| UncontrolledFormatString.cs:17:46:17:49 | access to local variable path |
| UncontrolledFormatString.cs:20:23:20:38 | "Do not do this" |
| UncontrolledFormatString.cs:23:46:23:61 | "Do not do this" |
| UncontrolledFormatString.cs:31:20:31:28 | access to property Text |
| UncontrolledFormatString.cs:31:23:31:31 | access to property Text |
| UncontrolledFormatStringBad.cs:9:25:9:47 | access to property QueryString |
| UncontrolledFormatStringBad.cs:12:39:12:44 | access to local variable format |
#select
| UncontrolledFormatString.cs:14:23:14:26 | access to local variable path | UncontrolledFormatString.cs:11:23:11:45 | access to property QueryString | UncontrolledFormatString.cs:14:23:14:26 | access to local variable path | $@ flows to here and is used as a format string. | UncontrolledFormatString.cs:11:23:11:45 | access to property QueryString | access to property QueryString |
| UncontrolledFormatString.cs:17:46:17:49 | access to local variable path | UncontrolledFormatString.cs:11:23:11:45 | access to property QueryString | UncontrolledFormatString.cs:17:46:17:49 | access to local variable path | $@ flows to here and is used as a format string. | UncontrolledFormatString.cs:11:23:11:45 | access to property QueryString | access to property QueryString |
| UncontrolledFormatString.cs:31:20:31:28 | access to property Text | UncontrolledFormatString.cs:31:20:31:28 | access to property Text | UncontrolledFormatString.cs:31:20:31:28 | access to property Text | $@ flows to here and is used as a format string. | UncontrolledFormatString.cs:31:20:31:28 | access to property Text | access to property Text |
| UncontrolledFormatString.cs:31:23:31:31 | access to property Text | UncontrolledFormatString.cs:31:23:31:31 | access to property Text | UncontrolledFormatString.cs:31:23:31:31 | access to property Text | $@ flows to here and is used as a format string. | UncontrolledFormatString.cs:31:23:31:31 | access to property Text | access to property Text |
| UncontrolledFormatStringBad.cs:12:39:12:44 | access to local variable format | UncontrolledFormatStringBad.cs:9:25:9:47 | access to property QueryString | UncontrolledFormatStringBad.cs:12:39:12:44 | access to local variable format | $@ flows to here and is used as a format string. | UncontrolledFormatStringBad.cs:9:25:9:47 | access to property QueryString | access to property QueryString |

View File

@@ -7,7 +7,7 @@ public class HttpHandler : IHttpHandler
public void ProcessRequest(HttpContext ctx)
{
string format = ctx.Request.QueryString["nameformat"];
// BAD: Uncontrolled format string.
FormattedName = string.Format(format, Surname, Forenames);
}

View File

@@ -43,7 +43,7 @@ namespace Newtonsoft.Json.Linq
public static explicit operator string(JToken t) => null;
public IEnumerable<JToken> SelectToken(string s) => null;
public IEnumerable<JToken> SelectToken(string s) => null;
}
public class JObject : JToken

View File

@@ -31,7 +31,7 @@ namespace System.Windows.Forms
public char PasswordChar { get; set; }
public bool UseSystemPasswordChar { get; set; }
}
class RichTextBox : TextBoxBase
{
public string Rtf => null;