diff --git a/csharp/ql/lib/semmle/code/csharp/security/dataflow/TypeNameHandlingQuery.qll b/csharp/ql/lib/semmle/code/csharp/security/dataflow/TypeNameHandlingQuery.qll new file mode 100644 index 000000000000..c456326174bc --- /dev/null +++ b/csharp/ql/lib/semmle/code/csharp/security/dataflow/TypeNameHandlingQuery.qll @@ -0,0 +1,319 @@ +/** + * Provides classes and predicates for detecting unsafe usage of + * Newtonsoft.Json `TypeNameHandling` settings. + * + * Setting `TypeNameHandling` to any value other than `None` during + * deserialization can enable remote code execution if untrusted data + * is deserialized without a custom `SerializationBinder`. + */ + +import csharp +import semmle.code.csharp.security.dataflow.flowsources.Remote +import semmle.code.csharp.serialization.Deserializers + +// --------------------------------------------------------------------------- +// Source expressions: unsafe TypeNameHandling values +// --------------------------------------------------------------------------- +/** + * An expression that represents an unsafe `TypeNameHandling` value — + * any member of the `TypeNameHandling` enum other than `None`, or an + * integer literal greater than zero (which maps to a non-`None` value). + */ +class BadTypeHandling extends Expr { + BadTypeHandling() { + exists(Enum e, EnumConstant c | + e.hasFullyQualifiedName("Newtonsoft.Json", "TypeNameHandling") and + c = e.getAnEnumConstant() and + this = c.getAnAccess() and + not c.hasName("None") + ) + or + this.(IntegerLiteral).getValue().toInt() > 0 + } +} + +// --------------------------------------------------------------------------- +// TypeNameHandling property modelling +// --------------------------------------------------------------------------- +/** + * The `TypeNameHandling` property on `JsonSerializerSettings` or `JsonSerializer`. + */ +class TypeNameHandlingProperty extends Property { + TypeNameHandlingProperty() { + this.hasFullyQualifiedName("Newtonsoft.Json", + ["JsonSerializerSettings", "JsonSerializer"], "TypeNameHandling") + } +} + +/** A write to a `TypeNameHandling` property. */ +class TypeNameHandlingPropertyWrite extends PropertyWrite { + TypeNameHandlingPropertyWrite() { this.getProperty() instanceof TypeNameHandlingProperty } + + /** Gets the right-hand side value assigned to this property. */ + Expr getAssignedValue() { + exists(AssignExpr a | + a.getLValue() = this and + result = a.getRValue() + ) + } +} + +/** + * A write to a `TypeNameHandling` property where the assigned value is + * known to be unsafe (i.e. not `None`). + */ +class BadTypeHandlingPropertyWrite extends TypeNameHandlingPropertyWrite { + BadTypeHandlingPropertyWrite() { + exists(BadTypeHandling b | + DataFlow::localExprFlow(b, this.getAssignedValue()) + ) + } +} + +// --------------------------------------------------------------------------- +// Binder property modelling +// --------------------------------------------------------------------------- +/** + * A write to the `SerializationBinder` or `Binder` property on + * `JsonSerializerSettings` or `JsonSerializer`. Setting a custom binder + * is a mitigation against unsafe `TypeNameHandling`. + */ +class BinderPropertyWrite extends PropertyWrite { + BinderPropertyWrite() { + this.getProperty() + .hasFullyQualifiedName("Newtonsoft.Json", + ["JsonSerializerSettings", "JsonSerializer"], + ["SerializationBinder", "Binder"]) + } +} + +// --------------------------------------------------------------------------- +// Deserialize call argument modelling +// --------------------------------------------------------------------------- +/** + * An argument passed to a `Newtonsoft.Json.JsonConvert.DeserializeObject` call. + */ +class DeserializeArg extends Expr { + MethodCall deserializeCall; + + DeserializeArg() { + deserializeCall.getTarget() instanceof NewtonsoftJsonConvertClassDeserializeObjectMethod and + deserializeCall.getAnArgument() = this + } + + /** Gets the enclosing `DeserializeObject` method call. */ + MethodCall getDeserializeCall() { result = deserializeCall } +} + +/** + * A `JsonSerializerSettings`-typed argument passed to a `DeserializeObject` + * call. This is the primary sink for unsafe `TypeNameHandling` flow. + */ +class JsonSerializerSettingsArg extends DeserializeArg { + JsonSerializerSettingsArg() { this.getType() instanceof JsonSerializerSettingsClass } +} + +// --------------------------------------------------------------------------- +// Taint tracking: remote input → DeserializeObject +// --------------------------------------------------------------------------- +/** + * Tracks tainted data flowing from remote sources into arguments of + * `JsonConvert.DeserializeObject`, including flow through `ToString()` calls. + */ +module UserInputToDeserializeObjectCallConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource } + + predicate isSink(DataFlow::Node sink) { + exists(MethodCall mc | + mc.getTarget().hasFullyQualifiedName("Newtonsoft.Json.JsonConvert", _) and + mc.getTarget().hasUndecoratedName("DeserializeObject") and + sink.asExpr() = mc.getAnArgument() + ) + } + + predicate isAdditionalFlowStep(DataFlow::Node pred, DataFlow::Node succ) { + exists(MethodCall ma | + ma.getTarget().hasName("ToString") and + ma.getQualifier() = pred.asExpr() and + succ.asExpr() = ma + ) + } +} + +module UserInputToDeserializeObjectCallFlow = + TaintTracking::Global; + +// --------------------------------------------------------------------------- +// Data flow: binder set on settings object +// --------------------------------------------------------------------------- +/** + * Tracks whether a `SerializationBinder` is assigned via an object + * initializer and the resulting settings object flows to a + * `DeserializeObject` call argument. + */ +module BinderConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node source) { + exists(ObjectCreation oc, MemberInitializer mi | + oc.getInitializer().(ObjectInitializer).getAMemberInitializer() = mi and + mi.getInitializedMember().hasName(["Binder", "SerializationBinder"]) and + source.asExpr() = oc + ) + } + + predicate isSink(DataFlow::Node sink) { sink.asExpr() instanceof JsonSerializerSettingsArg } +} + +module BinderFlow = DataFlow::Global; + +// --------------------------------------------------------------------------- +// Binder-set check (mitigation detection) +// --------------------------------------------------------------------------- +/** + * Holds if a custom `SerializationBinder` or `Binder` has been set on the + * settings object referenced by `arg`, either through an object initializer + * (tracked via `BinderFlow`) or through a later property write on the same + * variable. + */ +predicate hasBinderSet(JsonSerializerSettingsArg arg) { + // Binder was set in an object initializer and flowed to `arg` + exists(BinderFlow::PathNode sink | + sink.isSink() and + sink.getNode().asExpr() = arg + ) + or + // Binder was set via a property write on the same variable + exists(PropertyWrite pw | + pw.getProperty().hasName(["Binder", "SerializationBinder"]) and + pw.getQualifier().(Access).getTarget() = arg.(Access).getTarget() + ) +} + +// --------------------------------------------------------------------------- +// Sink node: TypeNameHandling property write value +// --------------------------------------------------------------------------- +/** + * A data-flow node representing the value assigned to a `TypeNameHandling` + * property. Provides a predicate to check whether a mitigating binder is set. + */ +class TypeNameHandlingPropertySink extends DataFlow::Node { + TypeNameHandlingPropertySink() { + exists(TypeNameHandlingPropertyWrite pw | + this.asExpr() = pw.getAssignedValue() + ) + } + + /** Holds if a custom binder is set on the same settings object. */ + predicate hasBinderSet() { + exists(JsonSerializerSettingsArg arg | + this.asExpr() = arg and + hasBinderSet(arg) + ) + } +} + +// --------------------------------------------------------------------------- +// Main flow: unsafe TypeNameHandling value → DeserializeObject settings arg +// --------------------------------------------------------------------------- +/** + * Tracks unsafe `TypeNameHandling` values flowing into `JsonSerializerSettings` + * arguments of `DeserializeObject` calls. Includes additional flow steps for + * integer-to-enum casts and property writes on settings objects. + */ +module UnsafeTypeNameHandlingFlowConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node source) { source.asExpr() instanceof BadTypeHandling } + + predicate isSink(DataFlow::Node sink) { sink.asExpr() instanceof JsonSerializerSettingsArg } + + predicate isBarrierIn(DataFlow::Node node) { isSource(node) } + + predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) { + // Cast from integer literal to TypeNameHandling enum + node1.asExpr() instanceof IntegerLiteral and + node2.asExpr().(CastExpr).getExpr() = node1.asExpr() + or + node1.getType() instanceof TypeNameHandlingEnum and + exists(TypeNameHandlingPropertyWrite pw, Assignment a | + a.getLValue() = pw and + ( + // Explicit property write: flow from the assigned value to the + // JsonSerializerSettingsArg that accesses the same settings variable + node1.asExpr() = a.getRValue() and + node2.asExpr().(JsonSerializerSettingsArg).(VariableAccess).getTarget() = + pw.getQualifier().(VariableAccess).getTarget() + or + // Object initializer: flow from the member initializer value to the + // ObjectCreation, which then flows locally to the JsonSerializerSettingsArg + exists(ObjectInitializer oi, ObjectCreation oc | + node1.asExpr() = oi.getAMemberInitializer().getRValue() and + oc.getInitializer() = oi and + DataFlow::localExprFlow(oc, node2.asExpr().(JsonSerializerSettingsArg)) + ) + ) + ) + } +} + +module UnsafeTypeNameHandlingFlow = DataFlow::Global; + +// --------------------------------------------------------------------------- +// Settings / serializer object creation modelling +// --------------------------------------------------------------------------- +/** + * An `ObjectCreation` of type `Newtonsoft.Json.JsonSerializerSettings` or + * `Newtonsoft.Json.JsonSerializer`. + */ +class JsonSerializerSettingsCreation extends ObjectCreation { + JsonSerializerSettingsCreation() { + this.getTarget() + .hasFullyQualifiedName("Newtonsoft.Json.JsonSerializerSettings", + "JsonSerializerSettings") + or + this.getTarget() + .hasFullyQualifiedName("Newtonsoft.Json.JsonSerializer", "JsonSerializer") + } + + /** Gets the type of the binder assigned to this settings object. */ + Class getAssignedBinderType() { + exists(AssignExpr ae | + ae.getLValue() = this.getBinderPropertyWrite() and + ae.getRValue().getType() = result + ) + } + + /** Gets a `BinderPropertyWrite` associated with this settings object. */ + BinderPropertyWrite getBinderPropertyWrite() { result = this.getPropertyWrite() } + + /** Gets a `TypeNameHandlingPropertyWrite` associated with this settings object. */ + TypeNameHandlingPropertyWrite getTypeNameHandlingPropertyWrite() { + result = this.getPropertyWrite() + } + + /** + * Gets a `PropertyWrite` associated with this settings object, via an + * initializer, direct local flow, or an assignment to the same property. + */ + PropertyWrite getPropertyWrite() { + result = this.getInitializer().getAChild*() + or + // Direct local flow via some intermediary + DataFlow::localExprFlow(this, result.getQualifier()) + or + // Local flow via property writes + this.hasPropertyWrite(result) + } + + /** + * Holds if `pw` is a property write on the same target that this object + * creation is assigned to, within the same callable. + */ + bindingset[pw] + pragma[inline_late] + predicate hasPropertyWrite(PropertyWrite pw) { + exists(Assignment a | + a.getRValue() = this and + a.getLValue().(PropertyAccess).getTarget() = + pw.getQualifier().(PropertyAccess).getTarget() and + a.getEnclosingCallable() = pw.getEnclosingCallable() + ) + } +} \ No newline at end of file diff --git a/csharp/ql/src/Security Features/CWE-502/UnsafeTypeNameHandling.qhelp b/csharp/ql/src/Security Features/CWE-502/UnsafeTypeNameHandling.qhelp new file mode 100644 index 000000000000..6dda55071951 --- /dev/null +++ b/csharp/ql/src/Security Features/CWE-502/UnsafeTypeNameHandling.qhelp @@ -0,0 +1,38 @@ + + + + +

This query checks for flow from a user input source to a Newtonsoft.Json DeserializeObject call that uses an unsafe TypeNameHandling setting and has a SerializationBinder set.

+ +

The Newtonsoft.Json.TypeNameHandling enumeration value of Auto or All is vulnerable when deserializing untrusted data. + An attacker could modify the serialized data to include unexpected types to inject objects with malicious side effects. + An attack against an insecure deserializer could, for example, execute commands on the underlying operating system, communicate over the network, or delete files.

+
+ + +

Use the TypeNameHandling value of None, or use a custom ISerializationBinder.

+
+ + + +

Violation:

+ + + +

Solution using TypeNameHandling.None:

+ + + +

Solution using custom ISerializationBinder:

+ + + +
+ + +
  • Microsoft: Do Not Use TypeNameHandling Values Other Than None.
  • +
    + +
    diff --git a/csharp/ql/src/Security Features/CWE-502/UnsafeTypeNameHandling.ql b/csharp/ql/src/Security Features/CWE-502/UnsafeTypeNameHandling.ql new file mode 100644 index 000000000000..f54e9711a61f --- /dev/null +++ b/csharp/ql/src/Security Features/CWE-502/UnsafeTypeNameHandling.ql @@ -0,0 +1,24 @@ +/** + * @name Unsafe TypeNameHandling value + * @description Checks for when JsonSerializer that also uses an unsafe TypeNameHandling setting + * @id cs/unsafe-type-name-handling + * @kind problem + * @problem.severity warning + * @precision high + * @tags security + * external/cwe/cwe-502 + */ + +import csharp +import semmle.code.csharp.security.dataflow.TypeNameHandlingQuery + +from + JsonSerializerSettingsCreation oc, TypeNameHandlingPropertyWrite pw, DataFlow::Node source, + DataFlow::Node sink +where + pw = oc.getTypeNameHandlingPropertyWrite() and + sink = DataFlow::exprNode(pw.getAssignedValue()) and + UnsafeTypeNameHandlingFlow::flow(source, sink) +select oc.getAssignedBinderType().getAMethod("BindToType"), + "This is the BindToType() implementation used in $@ with $@. Ensure that it checks the TypeName against an allow or deny list, and returns null or throws an exception when passed an unexpected type", oc, "JsonSerializerSettings", + pw.getAssignedValue(), "an unsafe TypeNameHandling value" diff --git a/csharp/ql/src/Security Features/CWE-502/UnsafeTypeNameHandlingInDefaultSetting.qhelp b/csharp/ql/src/Security Features/CWE-502/UnsafeTypeNameHandlingInDefaultSetting.qhelp new file mode 100644 index 000000000000..f8be8372db0e --- /dev/null +++ b/csharp/ql/src/Security Features/CWE-502/UnsafeTypeNameHandlingInDefaultSetting.qhelp @@ -0,0 +1,39 @@ + + + + +

    The Newtonsoft.Json.TypeNameHandling enumeration value of other than None is vulnerable when deserializing untrusted data. + An attacker could modify the serialized data to include unexpected types to inject objects with malicious side effects. + An attack against an insecure deserializer could, for example, execute commands on the underlying operating system, communicate over the network, or delete files.

    + +

    This query detects assignment of unsafe TypeNameHandling values to JsonConvert.DefaultSettings, and where these settings are used to deserialize user input

    + +
    + + +

    Use the TypeNameHandling value of None, or use a custom ISerializationBinder.

    +
    + + + +

    Violation:

    + + + +

    Solution using TypeNameHandling.None:

    + + + +

    Solution using custom ISerializationBinder:

    + + + +
    + + +
  • Microsoft: Do Not Use TypeNameHandling Values Other Than None.
  • +
    + +
    diff --git a/csharp/ql/src/Security Features/CWE-502/UnsafeTypeNameHandlingInDefaultSetting.ql b/csharp/ql/src/Security Features/CWE-502/UnsafeTypeNameHandlingInDefaultSetting.ql new file mode 100644 index 000000000000..8a920f5aceb0 --- /dev/null +++ b/csharp/ql/src/Security Features/CWE-502/UnsafeTypeNameHandlingInDefaultSetting.ql @@ -0,0 +1,48 @@ +/** + * @name Unsafe TypeNameHandling assigned to JsonConvert.DefaultSetting + * @description Using an unsafe TypeNameHandling constant is a security vulnerability. + * @kind problem + * @id cs/unsafe-type-name-handling-default-setting + * @problem.severity error + * @precision high + * @tags security + * external/cwe/cwe-502 + */ + +import csharp +import semmle.code.csharp.security.dataflow.TypeNameHandlingQuery +import semmle.code.csharp.dataflow.internal.DataFlowPrivate as DataFlowPrivate + +module TypeNameHandlingDefaultSettingsFlowConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node source) { + source.asExpr() instanceof JsonSerializerSettingsCreation + } + + predicate isSink(DataFlow::Node sink) { + exists(AssignExpr ae, PropertyWrite pw | + pw.getProperty().hasFullyQualifiedName("Newtonsoft.Json.JsonConvert", "DefaultSettings") and + sink.asExpr() = ae.getRValue() and + pw = ae.getLValue() + ) + } + + predicate isAdditionalFlowStep(DataFlow::Node pred, DataFlow::Node succ) { + // Step from the dataflow node representing the return value of the lambda to the lambda itself. + exists(LambdaExpr le | + pred.(DataFlowPrivate::ReturnNode).getEnclosingCallable() = le and + le = succ.asExpr() + ) + } +} + +module TypeNameHandlingDefaultSettingsFlow = + TaintTracking::Global; + +from DataFlow::Node badTypeNameHandling, DataFlow::Node typeNameHandlingPropertyWrite +where + TypeNameHandlingDefaultSettingsFlow::flow(badTypeNameHandling, typeNameHandlingPropertyWrite) and + // Detecting if there exists some flow from user input to a call that could use the unsafe settings + // Not including the actual flow in the results because doing so a cartesian product as the two flows are unrelated + UserInputToDeserializeObjectCallFlow::flow(_, _) +select badTypeNameHandling, + "Assignment of this TypeNameHandling constant to JsonConvert.DefaultSettings is unsafe" diff --git a/csharp/ql/src/Security Features/CWE-502/UnsafeTypeNameHandlingInStartup.qhelp b/csharp/ql/src/Security Features/CWE-502/UnsafeTypeNameHandlingInStartup.qhelp new file mode 100644 index 000000000000..560323a1f5ec --- /dev/null +++ b/csharp/ql/src/Security Features/CWE-502/UnsafeTypeNameHandlingInStartup.qhelp @@ -0,0 +1,39 @@ + + + + +

    The Newtonsoft.Json.TypeNameHandling enumeration value of other than None is vulnerable when deserializing untrusted data. + An attacker could modify the serialized data to include unexpected types to inject objects with malicious side effects. + An attack against an insecure deserializer could, for example, execute commands on the underlying operating system, communicate over the network, or delete files.

    + +

    This query detects assignment of unsafe TypeNameHandling values in Startup.cs using AddJsonOptions or AddNewtonsoftJson, and where these settings are used to deserialize user input

    + +
    + + +

    Use the TypeNameHandling value of None, or use a custom ISerializationBinder.

    +
    + + + +

    Violation:

    + + + +

    Solution using TypeNameHandling.None:

    + + + +

    Solution using custom ISerializationBinder:

    + + + +
    + + +
  • Microsoft: Do Not Use TypeNameHandling Values Other Than None.
  • +
    + +
    diff --git a/csharp/ql/src/Security Features/CWE-502/UnsafeTypeNameHandlingInStartup.ql b/csharp/ql/src/Security Features/CWE-502/UnsafeTypeNameHandlingInStartup.ql new file mode 100644 index 000000000000..7ea10b1fe857 --- /dev/null +++ b/csharp/ql/src/Security Features/CWE-502/UnsafeTypeNameHandlingInStartup.ql @@ -0,0 +1,35 @@ +/** + * @name Unsafe TypeNameHandling in Startup + * @description Using an unsafe TypeNameHandling constant is a security vulnerability. + * @kind problem + * @id cs/unsafe-type-name-handling-in-startup + * @problem.severity error + * @precision high + * @tags security + * external/cwe/cwe-502 + */ + +import csharp +import semmle.code.csharp.security.dataflow.TypeNameHandlingQuery + +class AddJsonOptionsCall extends MethodCall { + AddJsonOptionsCall() { + this.getTarget() + .hasFullyQualifiedName("Microsoft.Extensions.DependencyInjection.NewtonsoftJsonMvcBuilderExtensions", + "AddNewtonsoftJson") or + this.getTarget() + .hasFullyQualifiedName("Microsoft.Extensions.DependencyInjection.MvcJsonMvcBuilderExtensions", + "AddJsonOptions") + } +} + +from + TypeNameHandlingPropertyWrite pw, AddJsonOptionsCall addJsonOptions, + DataFlow::Node badTypeNameHandling, DataFlow::Node typeNameHandlingPropertyWrite +where + addJsonOptions.getEnclosingCallable().hasName("ConfigureServices") and + addJsonOptions.getAChild*() = pw and + typeNameHandlingPropertyWrite = DataFlow::exprNode(pw.getAssignedValue()) and + UnsafeTypeNameHandlingFlow::flow(badTypeNameHandling, typeNameHandlingPropertyWrite) and + UserInputToDeserializeObjectCallFlow::flow(_, _) +select badTypeNameHandling, "Use of this TypeNameHandling constant in Startup.cs is unsafe" diff --git a/csharp/ql/src/Security Features/CWE-502/UnsafeTypeNameHandlingWithoutBinder.qhelp b/csharp/ql/src/Security Features/CWE-502/UnsafeTypeNameHandlingWithoutBinder.qhelp new file mode 100644 index 000000000000..e3851983616d --- /dev/null +++ b/csharp/ql/src/Security Features/CWE-502/UnsafeTypeNameHandlingWithoutBinder.qhelp @@ -0,0 +1,39 @@ + + + + +

    This query checks for flow from a user input source to a Newtonsoft.Json DeserializeObject call that uses an unsafe TypeNameHandling setting and has no SerializationBinder set.

    + + +

    The Newtonsoft.Json.TypeNameHandling enumeration value of Auto or All is vulnerable when deserializing untrusted data. + An attacker could modify the serialized data to include unexpected types to inject objects with malicious side effects. + An attack against an insecure deserializer could, for example, execute commands on the underlying operating system, communicate over the network, or delete files.

    +
    + + +

    Use the TypeNameHandling value of None, or use a custom ISerializationBinder.

    +
    + + + +

    Violation:

    + + + +

    Solution using TypeNameHandling.None:

    + + + +

    Solution using custom ISerializationBinder:

    + + + +
    + + +
  • Microsoft: Do Not Use TypeNameHandling Values Other Than None.
  • +
    + +
    diff --git a/csharp/ql/src/Security Features/CWE-502/UnsafeTypeNameHandlingWithoutBinder.ql b/csharp/ql/src/Security Features/CWE-502/UnsafeTypeNameHandlingWithoutBinder.ql new file mode 100644 index 000000000000..01f2ba301dc3 --- /dev/null +++ b/csharp/ql/src/Security Features/CWE-502/UnsafeTypeNameHandlingWithoutBinder.ql @@ -0,0 +1,28 @@ +/** + * @name Unsafe TypeNameHandling without Binder to deserialize user input + * @description Using an unsafe TypeNameHandling constant is a security vulnerability. + * @kind path-problem + * @id cs/unsafe-type-name-handling-without-binder + * @problem.severity error + * @precision high + * @tags security + * external/cwe/cwe-502 + */ + +import csharp +import semmle.code.csharp.serialization.Deserializers +import semmle.code.csharp.security.dataflow.TypeNameHandlingQuery + +import UserInputToDeserializeObjectCallFlow::PathGraph + +from +UserInputToDeserializeObjectCallFlow::PathNode userInput, UserInputToDeserializeObjectCallFlow::PathNode deserializeArg, +DataFlow::Node badTypeNameHandling, DataFlow::Node typeNameHandlingSettingArg, +BadTypeHandling bth +where +UnsafeTypeNameHandlingFlow::flow(badTypeNameHandling, typeNameHandlingSettingArg) and +not hasBinderSet(typeNameHandlingSettingArg.asExpr().(JsonSerializerSettingsArg)) and +UserInputToDeserializeObjectCallFlow::flowPath(userInput, deserializeArg) and +deserializeArg.getNode().asExpr().(DeserializeArg).getDeserializeCall() = typeNameHandlingSettingArg.asExpr().(JsonSerializerSettingsArg).getDeserializeCall() and +bth = badTypeNameHandling.asExpr() +select deserializeArg.getNode(), userInput, deserializeArg, "Use of $@ constant to deserialize user-controlled input is unsafe", bth, "this Json.net TypeNameHandling" \ No newline at end of file diff --git a/csharp/ql/src/Security Features/CWE-502/examples/UnsafeTypeNameHandlingBad.cs b/csharp/ql/src/Security Features/CWE-502/examples/UnsafeTypeNameHandlingBad.cs new file mode 100644 index 000000000000..2ea0e401961d --- /dev/null +++ b/csharp/ql/src/Security Features/CWE-502/examples/UnsafeTypeNameHandlingBad.cs @@ -0,0 +1,10 @@ +using Newtonsoft.Json; +public class ExampleClass +{ + public JsonSerializerSettings Settings { get; } + public ExampleClass() + { + Settings = new JsonSerializerSettings(); + Settings.TypeNameHandling = TypeNameHandling.All; // BAD + } +} diff --git a/csharp/ql/src/Security Features/CWE-502/examples/UnsafeTypeNameHandlingGood.cs b/csharp/ql/src/Security Features/CWE-502/examples/UnsafeTypeNameHandlingGood.cs new file mode 100644 index 000000000000..f63b268cf756 --- /dev/null +++ b/csharp/ql/src/Security Features/CWE-502/examples/UnsafeTypeNameHandlingGood.cs @@ -0,0 +1,9 @@ +using Newtonsoft.Json; +public class ExampleClass +{ + public JsonSerializerSettings Settings { get; } + public ExampleClass() + { + Settings = new JsonSerializerSettings(); // GOOD: The default value of Settings.TypeNameHandling is TypeNameHandling.None. + } +} diff --git a/csharp/ql/src/Security Features/CWE-502/examples/UnsafeTypeNameHandlingWithBinderGood.cs b/csharp/ql/src/Security Features/CWE-502/examples/UnsafeTypeNameHandlingWithBinderGood.cs new file mode 100644 index 000000000000..aedc0fe4cf18 --- /dev/null +++ b/csharp/ql/src/Security Features/CWE-502/examples/UnsafeTypeNameHandlingWithBinderGood.cs @@ -0,0 +1,43 @@ +using Newtonsoft.Json; +using Newtonsoft.Json.Serialization; +using System; +using System.Collections.Generic; +using System.Linq; +using System.Text; +using System.Threading.Tasks; + +namespace TypeHandlingSample +{ + public class KnownTypesBinder : ISerializationBinder + { + public IList KnownTypes { get; set; } + + public Type BindToType(string assemblyName, string typeName) + { + return KnownTypes.SingleOrDefault(t => t.Name == typeName); + } + + public void BindToName(Type serializedType, out string assemblyName, out string typeName) + { + assemblyName = null; + typeName = serializedType.Name; + } + } + public class SomeClass + { + public string Name { get; set; } + } + + internal class Okay + { + public JsonSerializerSettings Settings { get; } + public KnownTypesBinder Binder { get; } + public Okay() + { + Settings = new JsonSerializerSettings(); + Binder = new KnownTypesBinder { KnownTypes = new List { typeof(SomeClass) } }; + Settings.SerializationBinder = Binder; + Settings.TypeNameHandling = TypeNameHandling.All; // Okay, because SerializationBinder set to custom ISerializationBinder + } + } +} diff --git a/csharp/ql/test/query-tests/Security Features/CWE-502/UnsafeTypeNameHandling/Test.cs b/csharp/ql/test/query-tests/Security Features/CWE-502/UnsafeTypeNameHandling/Test.cs new file mode 100644 index 000000000000..1afaa6527527 --- /dev/null +++ b/csharp/ql/test/query-tests/Security Features/CWE-502/UnsafeTypeNameHandling/Test.cs @@ -0,0 +1,94 @@ +using System; +using System.Web; +using System.Text; +using System.Collections.Generic; +using Newtonsoft.Json; +using Newtonsoft.Json.Serialization; +using System.Threading.Tasks; + +namespace TypeHandlingSample +{ + public class BadBinder : ISerializationBinder + { + public void BindToName(Type serializedType, out string assemblyName, out string typeName) + { + assemblyName = serializedType.Assembly.FullName; + typeName = serializedType.Name; + } + + public Type BindToType(string assemblyName, string typeName) // $ Alert + { + return Type.GetType(typeName); + } + } + + public class OkayBinder : DefaultSerializationBinder + { + public List okayTypes; + public override Type BindToType(string? assemblyName, string typeName) // $ Alert + { + if (okayTypes.Contains(typeName)) + { + return Type.GetType(typeName); + + } + else + { + throw new Exception("unexpected type"); + } + } + } + + public class OkayBinder2 : ISerializationBinder + { + private readonly ISerializationBinder innerBinder; + + public void BindToName(Type serializedType, out string? assemblyName, out string? typeName) + { + this.innerBinder.BindToName(serializedType, out assemblyName, out typeName); + } + public Type BindToType(string? assemblyName, string typeName) // $ Alert + { + return this.innerBinder.BindToType(assemblyName, typeName); + } + } + + public class Test + { + public JsonSerializerSettings SettingsBad { get; } + + public JsonSerializerSettings SettingsOkay { get; } + + public JsonSerializerSettings SettingsOkay2 { get; } + + public BadBinder badBinder { get; } + + public OkayBinder okayBinder { get; } + + public OkayBinder2 okayBinder2 { get; } + public Test() + { + //Bad, custom binder that does not check type + SettingsBad = new JsonSerializerSettings() + { + SerializationBinder = badBinder, + TypeNameHandling = TypeNameHandling.All + }; + + //OKAY, custom binder that checks type + SettingsOkay = new JsonSerializerSettings() + { + SerializationBinder = okayBinder, + TypeNameHandling = TypeNameHandling.All + }; + + //OKAY, custom binder that returns BindToType() from a member binder + SettingsOkay2 = new JsonSerializerSettings() + { + SerializationBinder = okayBinder2, + TypeNameHandling = TypeNameHandling.All + }; + } + } + +} \ No newline at end of file diff --git a/csharp/ql/test/query-tests/Security Features/CWE-502/UnsafeTypeNameHandling/UnsafeTypeNameHandling.expected b/csharp/ql/test/query-tests/Security Features/CWE-502/UnsafeTypeNameHandling/UnsafeTypeNameHandling.expected new file mode 100644 index 000000000000..7daf2fb8f7cb --- /dev/null +++ b/csharp/ql/test/query-tests/Security Features/CWE-502/UnsafeTypeNameHandling/UnsafeTypeNameHandling.expected @@ -0,0 +1,3 @@ +| Test.cs:19:21:19:30 | BindToType | This is the BindToType() implementation used in $@ with $@. Ensure that it checks the TypeName against an allow or deny list, and returns null or throws an exception when passed an unexpected type | Test.cs:72:27:76:13 | object creation of type JsonSerializerSettings | JsonSerializerSettings | Test.cs:75:36:75:55 | access to constant All | an unsafe TypeNameHandling value | +| Test.cs:28:30:28:39 | BindToType | This is the BindToType() implementation used in $@ with $@. Ensure that it checks the TypeName against an allow or deny list, and returns null or throws an exception when passed an unexpected type | Test.cs:79:28:83:13 | object creation of type JsonSerializerSettings | JsonSerializerSettings | Test.cs:82:36:82:55 | access to constant All | an unsafe TypeNameHandling value | +| Test.cs:50:21:50:30 | BindToType | This is the BindToType() implementation used in $@ with $@. Ensure that it checks the TypeName against an allow or deny list, and returns null or throws an exception when passed an unexpected type | Test.cs:86:29:90:13 | object creation of type JsonSerializerSettings | JsonSerializerSettings | Test.cs:89:36:89:55 | access to constant All | an unsafe TypeNameHandling value | diff --git a/csharp/ql/test/query-tests/Security Features/CWE-502/UnsafeTypeNameHandling/UnsafeTypeNameHandling.qlref b/csharp/ql/test/query-tests/Security Features/CWE-502/UnsafeTypeNameHandling/UnsafeTypeNameHandling.qlref new file mode 100644 index 000000000000..54b8ed3901eb --- /dev/null +++ b/csharp/ql/test/query-tests/Security Features/CWE-502/UnsafeTypeNameHandling/UnsafeTypeNameHandling.qlref @@ -0,0 +1,3 @@ +query: Security Features/CWE-502/UnsafeTypeNameHandling.ql +postprocess: +- utils/test/InlineExpectationsTestQuery.ql diff --git a/csharp/ql/test/query-tests/Security Features/CWE-502/UnsafeTypeNameHandling/options b/csharp/ql/test/query-tests/Security Features/CWE-502/UnsafeTypeNameHandling/options new file mode 100644 index 000000000000..ed130f17969c --- /dev/null +++ b/csharp/ql/test/query-tests/Security Features/CWE-502/UnsafeTypeNameHandling/options @@ -0,0 +1,3 @@ +semmle-extractor-options: /nostdlib /noconfig +semmle-extractor-options: --load-sources-from-project:${testdir}/../../../../resources/stubs/_frameworks/Microsoft.NETCore.App/Microsoft.NETCore.App.csproj +semmle-extractor-options: --load-sources-from-project:${testdir}/../../../../resources/stubs/Newtonsoft.Json/13.0.4/Newtonsoft.Json.csproj diff --git a/csharp/ql/test/query-tests/Security Features/CWE-502/UnsafeTypeNameHandlingInDefaultSetting/Test.cs b/csharp/ql/test/query-tests/Security Features/CWE-502/UnsafeTypeNameHandlingInDefaultSetting/Test.cs new file mode 100644 index 000000000000..9e58a9794c62 --- /dev/null +++ b/csharp/ql/test/query-tests/Security Features/CWE-502/UnsafeTypeNameHandlingInDefaultSetting/Test.cs @@ -0,0 +1,39 @@ +using System; +using Microsoft.AspNetCore.Http; +using Newtonsoft.Json; + +namespace TypeHandlingSample +{ + public class Record + { + public int Id { get; set; } + public string Name { get; set; } + } + public class Class1 + { + public void SetDefaultSettings(){ + var settings = new JsonSerializerSettings + { + TypeNameHandling = TypeNameHandling.Objects + }; // $ Alert + + JsonConvert.DefaultSettings = () => settings; + } + + public void SetDefaultSettings2() + { + JsonConvert.DefaultSettings = () => new JsonSerializerSettings + { + TypeNameHandling = TypeNameHandling.Objects + }; // $ Alert + } + } + + public class Class2 + { + public void DeserializeUserInput(HttpRequest req){ + string untrustedInput = req.Body.ToString(); + var deserializedRecord = JsonConvert.DeserializeObject(untrustedInput); + } + } +} diff --git a/csharp/ql/test/query-tests/Security Features/CWE-502/UnsafeTypeNameHandlingInDefaultSetting/UnsafeTypeNameHandlingInDefaultSetting.expected b/csharp/ql/test/query-tests/Security Features/CWE-502/UnsafeTypeNameHandlingInDefaultSetting/UnsafeTypeNameHandlingInDefaultSetting.expected new file mode 100644 index 000000000000..6da61e6af68a --- /dev/null +++ b/csharp/ql/test/query-tests/Security Features/CWE-502/UnsafeTypeNameHandlingInDefaultSetting/UnsafeTypeNameHandlingInDefaultSetting.expected @@ -0,0 +1,2 @@ +| Test.cs:15:28:18:13 | object creation of type JsonSerializerSettings | Assignment of this TypeNameHandling constant to JsonConvert.DefaultSettings is unsafe | +| Test.cs:25:49:28:13 | object creation of type JsonSerializerSettings | Assignment of this TypeNameHandling constant to JsonConvert.DefaultSettings is unsafe | diff --git a/csharp/ql/test/query-tests/Security Features/CWE-502/UnsafeTypeNameHandlingInDefaultSetting/UnsafeTypeNameHandlingInDefaultSetting.qlref b/csharp/ql/test/query-tests/Security Features/CWE-502/UnsafeTypeNameHandlingInDefaultSetting/UnsafeTypeNameHandlingInDefaultSetting.qlref new file mode 100644 index 000000000000..4e7084018a4f --- /dev/null +++ b/csharp/ql/test/query-tests/Security Features/CWE-502/UnsafeTypeNameHandlingInDefaultSetting/UnsafeTypeNameHandlingInDefaultSetting.qlref @@ -0,0 +1,3 @@ +query: Security Features/CWE-502/UnsafeTypeNameHandlingInDefaultSetting.ql +postprocess: +- utils/test/InlineExpectationsTestQuery.ql diff --git a/csharp/ql/test/query-tests/Security Features/CWE-502/UnsafeTypeNameHandlingInDefaultSetting/options b/csharp/ql/test/query-tests/Security Features/CWE-502/UnsafeTypeNameHandlingInDefaultSetting/options new file mode 100644 index 000000000000..e1b4d7c360e6 --- /dev/null +++ b/csharp/ql/test/query-tests/Security Features/CWE-502/UnsafeTypeNameHandlingInDefaultSetting/options @@ -0,0 +1,4 @@ +semmle-extractor-options: /nostdlib /noconfig +semmle-extractor-options: --load-sources-from-project:${testdir}/../../../../resources/stubs/Newtonsoft.Json/13.0.4/Newtonsoft.Json.csproj +semmle-extractor-options: --load-sources-from-project:${testdir}/../../../../resources/stubs/_frameworks/Microsoft.NETCore.App/Microsoft.NETCore.App.csproj +semmle-extractor-options: --load-sources-from-project:${testdir}/../../../../resources/stubs/_frameworks/Microsoft.AspNetCore.App/Microsoft.AspNetCore.App.csproj diff --git a/csharp/ql/test/query-tests/Security Features/CWE-502/UnsafeTypeNameHandlingInStartup/Test.cs b/csharp/ql/test/query-tests/Security Features/CWE-502/UnsafeTypeNameHandlingInStartup/Test.cs new file mode 100644 index 000000000000..ebb45e0f7fde --- /dev/null +++ b/csharp/ql/test/query-tests/Security Features/CWE-502/UnsafeTypeNameHandlingInStartup/Test.cs @@ -0,0 +1,38 @@ +using System; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.AspNetCore.Http; +using Newtonsoft.Json; + +namespace TypeHandlingSample +{ + + public class Startup + { + public void ConfigureServices(IServiceCollection services) + { + services.AddControllers().AddJsonOptions(options => + { + options.SerializerSettings.TypeNameHandling = Newtonsoft.Json.TypeNameHandling.Auto; // $ Alert + }); + services.AddControllers() + .AddNewtonsoftJson(options => + { + options.SerializerSettings.TypeNameHandling = TypeNameHandling.Objects; // $ Alert + }); + } + } + + public class Record + { + public int Id { get; set; } + public string Name { get; set; } + } + + public class SomeClass + { + public void DoDeserialize(HttpRequest req){ + string userInput = req.Body.ToString(); + Record json = JsonConvert.DeserializeObject(userInput); + } + } +} diff --git a/csharp/ql/test/query-tests/Security Features/CWE-502/UnsafeTypeNameHandlingInStartup/UnsafeTypeNameHandlingInStartup.expected b/csharp/ql/test/query-tests/Security Features/CWE-502/UnsafeTypeNameHandlingInStartup/UnsafeTypeNameHandlingInStartup.expected new file mode 100644 index 000000000000..65712b76fa25 --- /dev/null +++ b/csharp/ql/test/query-tests/Security Features/CWE-502/UnsafeTypeNameHandlingInStartup/UnsafeTypeNameHandlingInStartup.expected @@ -0,0 +1,2 @@ +| Test.cs:15:63:15:99 | access to constant Auto | Use of this TypeNameHandling constant in Startup.cs is unsafe | +| Test.cs:20:64:20:87 | access to constant Objects | Use of this TypeNameHandling constant in Startup.cs is unsafe | diff --git a/csharp/ql/test/query-tests/Security Features/CWE-502/UnsafeTypeNameHandlingInStartup/UnsafeTypeNameHandlingInStartup.qlref b/csharp/ql/test/query-tests/Security Features/CWE-502/UnsafeTypeNameHandlingInStartup/UnsafeTypeNameHandlingInStartup.qlref new file mode 100644 index 000000000000..bc4968f5cd56 --- /dev/null +++ b/csharp/ql/test/query-tests/Security Features/CWE-502/UnsafeTypeNameHandlingInStartup/UnsafeTypeNameHandlingInStartup.qlref @@ -0,0 +1,3 @@ +query: Security Features/CWE-502/UnsafeTypeNameHandlingInStartup.ql +postprocess: +- utils/test/InlineExpectationsTestQuery.ql diff --git a/csharp/ql/test/query-tests/Security Features/CWE-502/UnsafeTypeNameHandlingInStartup/options b/csharp/ql/test/query-tests/Security Features/CWE-502/UnsafeTypeNameHandlingInStartup/options new file mode 100644 index 000000000000..3e8025742171 --- /dev/null +++ b/csharp/ql/test/query-tests/Security Features/CWE-502/UnsafeTypeNameHandlingInStartup/options @@ -0,0 +1,2 @@ +semmle-extractor-options: /nostdlib /noconfig +semmle-extractor-options: --load-sources-from-project:${testdir}/../../../../resources/stubs/Newtonsoft.Json/13.0.4/Newtonsoft.Json.csproj diff --git a/csharp/ql/test/query-tests/Security Features/CWE-502/UnsafeTypeNameHandlingInStartup/stubs.cs b/csharp/ql/test/query-tests/Security Features/CWE-502/UnsafeTypeNameHandlingInStartup/stubs.cs new file mode 100644 index 000000000000..fb143ee4221f --- /dev/null +++ b/csharp/ql/test/query-tests/Security Features/CWE-502/UnsafeTypeNameHandlingInStartup/stubs.cs @@ -0,0 +1,61 @@ +using System; +using Newtonsoft.Json; + +namespace Microsoft.AspNetCore.Http{ + public class HttpRequest{ + public string Body; + } +} + +namespace Microsoft.Extensions.DependencyInjection +{ + public interface IServiceCollection + { + + } + + public interface IMvcBuilder + { + + } + + public class MvcBuilder : IMvcBuilder + { + } + + public static class ServiceCollectionExtensions + { + public static IMvcBuilder AddControllers(this IServiceCollection services) + { + return new MvcBuilder(); + } + } + + public static class MvcJsonMvcBuilderExtensions + { + public static IMvcBuilder AddJsonOptions(this IMvcBuilder builder, Action setupAction) + { + setupAction?.Invoke(new JsonOptions()); + return builder; + } + } + + public static class NewtonsoftJsonMvcBuilderExtensions + { + public static IMvcBuilder AddNewtonsoftJson(this IMvcBuilder builder, Action setupAction) + { + setupAction?.Invoke(new NewtonsoftJsonOptions()); + return builder; + } + } + + public class JsonOptions + { + public JsonSerializerSettings SerializerSettings { get; set; } = new JsonSerializerSettings(); + } + + public class NewtonsoftJsonOptions + { + public JsonSerializerSettings SerializerSettings { get; set; } = new JsonSerializerSettings(); + } +} diff --git a/csharp/ql/test/query-tests/Security Features/CWE-502/UnsafeTypeNameHandlingWithoutBinder/Test.cs b/csharp/ql/test/query-tests/Security Features/CWE-502/UnsafeTypeNameHandlingWithoutBinder/Test.cs new file mode 100644 index 000000000000..e91b9eea539c --- /dev/null +++ b/csharp/ql/test/query-tests/Security Features/CWE-502/UnsafeTypeNameHandlingWithoutBinder/Test.cs @@ -0,0 +1,143 @@ +using System; +using System.Collections; +using Newtonsoft.Json; +using Newtonsoft.Json.Serialization; +using Microsoft.AspNetCore.Http; + +namespace TypeHandlingSample +{ + public class Student{ + public string Name {get; set;} + public int ID {get; set;} + public object someObject{get; set;} + } + + public class CustomSerializationBinder : DefaultSerializationBinder + { + public override Type BindToType(string assemblyName, string typeName) + { + return base.BindToType(assemblyName, typeName); + } + } + + public class Test + { + JsonSerializerSettings SettingsOkay11 = new JsonSerializerSettings() + { + TypeNameHandling = TypeNameHandling.Auto, + SerializationBinder = new CustomSerializationBinder() + }; + + public Test(HttpRequest req, ISerializationBinder someBinder) + { + JsonSerializerSettings SettingsBad1 = new JsonSerializerSettings(); + //BAD + SettingsBad1.TypeNameHandling = TypeNameHandling.All; + + //BAD + JsonSerializerSettings SettingsBad2 = new JsonSerializerSettings() + { + TypeNameHandling = TypeNameHandling.All + }; + + //OKAY, set custom binder in initializer + JsonSerializerSettings SettingsOkay = new JsonSerializerSettings() + { + SerializationBinder = someBinder, + TypeNameHandling = TypeNameHandling.All + }; + + //OKAY, set custom binder in initializer + JsonSerializerSettings SettingsOkay1 = new JsonSerializerSettings() + { + TypeNameHandling = TypeNameHandling.All, + SerializationBinder = someBinder, + }; + + JsonSerializerSettings SettingsOkay2 = new JsonSerializerSettings(); + SettingsOkay2.SerializationBinder = someBinder; + // OKAY, because SerializationBinder set to custom ISerializationBinder before typenamehandling set + SettingsOkay2.TypeNameHandling = TypeNameHandling.All; + + + //OKAY, SerializationBinder set to custom binder + JsonSerializerSettings SettingsOkay3 = new JsonSerializerSettings{ + TypeNameHandling = TypeNameHandling.All, + SerializationBinder = someBinder + }; + + JsonSerializerSettings SettingsOkay4= new JsonSerializerSettings{ + SerializationBinder = someBinder + }; + SettingsOkay4.TypeNameHandling = TypeNameHandling.All; + + //OKAY, SerializationBinder set to custom binder + JsonSerializerSettings SettingsOkay5 = new JsonSerializerSettings(); + SettingsOkay5.SerializationBinder = someBinder; + SettingsOkay5.TypeNameHandling = TypeNameHandling.All; + + //OKAY, SerializationBinder set to custom binder in initializer + JsonSerializerSettings SettingsOkay6 = new JsonSerializerSettings{ + SerializationBinder = someBinder + }; + SettingsOkay6.TypeNameHandling = TypeNameHandling.All; + + //OKAY, default constructor safe by default + JsonSerializerSettings SettingsOkay7 = new JsonSerializerSettings(); + + //OKAY, TypeNameHandling set to None + JsonSerializerSettings SettingsOkay8 = new JsonSerializerSettings() + { + TypeNameHandling = TypeNameHandling.None + }; + + //OKAY, SerializationBinder set to custom binder in in following line + JsonSerializerSettings SettingsOkay9 = new JsonSerializerSettings{ + TypeNameHandling = TypeNameHandling.All + }; + SettingsOkay9.SerializationBinder = someBinder; + + var badValue = TypeNameHandling.Objects; + + //BAD, TypeNameHandling set to All + JsonSerializerSettings SettingsBad3 = new JsonSerializerSettings() + { + TypeNameHandling = badValue + }; + + var goodValue = TypeNameHandling.None; + + //OKAY, TypeNameHandling set to None + JsonSerializerSettings SettingsOkay10 = new JsonSerializerSettings() + { + TypeNameHandling = goodValue + }; + + string userInput = req.Body.ToString(); // $ Source + + var deserializedBad1 = JsonConvert.DeserializeObject(userInput, SettingsBad1); // $ Alert + var deserializedBad2 = JsonConvert.DeserializeObject(userInput, SettingsBad2); // $ Alert + var deserializedBad3 = JsonConvert.DeserializeObject(userInput, SettingsBad3); // $ Alert + + var deserializedOkay1 = JsonConvert.DeserializeObject(userInput, SettingsOkay1); + var deserializedOkay2 = JsonConvert.DeserializeObject(userInput, SettingsOkay2); + var deserializedOkay3 = JsonConvert.DeserializeObject(userInput, SettingsOkay3); + var deserializedOkay4 = JsonConvert.DeserializeObject(userInput, SettingsOkay4); + var deserializedOkay5 = JsonConvert.DeserializeObject(userInput, SettingsOkay5); + var deserializedOkay6 = JsonConvert.DeserializeObject(userInput, SettingsOkay6); + var deserializedOkay7 = JsonConvert.DeserializeObject(userInput, SettingsOkay7); + var deserializedOkay8 = JsonConvert.DeserializeObject(userInput, SettingsOkay8); + var deserializedOkay9 = JsonConvert.DeserializeObject(userInput, SettingsOkay9); + var deserializedOkay10 = JsonConvert.DeserializeObject(userInput, SettingsOkay10); + var deserializedOkay11 = JsonConvert.DeserializeObject(userInput, SettingsOkay11); + + } + + public void NotUserInput(string input){ + JsonSerializerSettings SettingsBad = new JsonSerializerSettings(); + //BAD + SettingsBad.TypeNameHandling = TypeNameHandling.All; + var deserializedOkay = JsonConvert.DeserializeObject(input, SettingsBad); //not from user input + } + } +} diff --git a/csharp/ql/test/query-tests/Security Features/CWE-502/UnsafeTypeNameHandlingWithoutBinder/UnsafeTypeNameHandlingWithoutBinder.expected b/csharp/ql/test/query-tests/Security Features/CWE-502/UnsafeTypeNameHandlingWithoutBinder/UnsafeTypeNameHandlingWithoutBinder.expected new file mode 100644 index 000000000000..03c4d88b66aa --- /dev/null +++ b/csharp/ql/test/query-tests/Security Features/CWE-502/UnsafeTypeNameHandlingWithoutBinder/UnsafeTypeNameHandlingWithoutBinder.expected @@ -0,0 +1,40 @@ +#select +| Test.cs:118:75:118:83 | access to local variable userInput | Test.cs:116:32:116:39 | access to property Body : Stream | Test.cs:118:75:118:83 | access to local variable userInput | Use of $@ constant to deserialize user-controlled input is unsafe | Test.cs:35:45:35:64 | access to constant All | this Json.net TypeNameHandling | +| Test.cs:119:75:119:83 | access to local variable userInput | Test.cs:116:32:116:39 | access to property Body : Stream | Test.cs:119:75:119:83 | access to local variable userInput | Use of $@ constant to deserialize user-controlled input is unsafe | Test.cs:40:36:40:55 | access to constant All | this Json.net TypeNameHandling | +| Test.cs:120:75:120:83 | access to local variable userInput | Test.cs:116:32:116:39 | access to property Body : Stream | Test.cs:120:75:120:83 | access to local variable userInput | Use of $@ constant to deserialize user-controlled input is unsafe | Test.cs:100:28:100:51 | access to constant Objects | this Json.net TypeNameHandling | +edges +| Test.cs:116:20:116:28 | access to local variable userInput : String | Test.cs:118:75:118:83 | access to local variable userInput | provenance | | +| Test.cs:116:20:116:28 | access to local variable userInput : String | Test.cs:119:75:119:83 | access to local variable userInput | provenance | | +| Test.cs:116:20:116:28 | access to local variable userInput : String | Test.cs:120:75:120:83 | access to local variable userInput | provenance | | +| Test.cs:116:20:116:28 | access to local variable userInput : String | Test.cs:122:76:122:84 | access to local variable userInput | provenance | | +| Test.cs:116:20:116:28 | access to local variable userInput : String | Test.cs:123:76:123:84 | access to local variable userInput | provenance | | +| Test.cs:116:20:116:28 | access to local variable userInput : String | Test.cs:124:76:124:84 | access to local variable userInput | provenance | | +| Test.cs:116:20:116:28 | access to local variable userInput : String | Test.cs:125:76:125:84 | access to local variable userInput | provenance | | +| Test.cs:116:20:116:28 | access to local variable userInput : String | Test.cs:126:76:126:84 | access to local variable userInput | provenance | | +| Test.cs:116:20:116:28 | access to local variable userInput : String | Test.cs:127:76:127:84 | access to local variable userInput | provenance | | +| Test.cs:116:20:116:28 | access to local variable userInput : String | Test.cs:128:76:128:84 | access to local variable userInput | provenance | | +| Test.cs:116:20:116:28 | access to local variable userInput : String | Test.cs:129:76:129:84 | access to local variable userInput | provenance | | +| Test.cs:116:20:116:28 | access to local variable userInput : String | Test.cs:130:76:130:84 | access to local variable userInput | provenance | | +| Test.cs:116:20:116:28 | access to local variable userInput : String | Test.cs:131:77:131:85 | access to local variable userInput | provenance | | +| Test.cs:116:20:116:28 | access to local variable userInput : String | Test.cs:132:77:132:85 | access to local variable userInput | provenance | | +| Test.cs:116:32:116:39 | access to property Body : Stream | Test.cs:116:32:116:50 | call to method ToString : String | provenance | Config | +| Test.cs:116:32:116:50 | call to method ToString : String | Test.cs:116:20:116:28 | access to local variable userInput : String | provenance | | +nodes +| Test.cs:116:20:116:28 | access to local variable userInput : String | semmle.label | access to local variable userInput : String | +| Test.cs:116:32:116:39 | access to property Body : Stream | semmle.label | access to property Body : Stream | +| Test.cs:116:32:116:50 | call to method ToString : String | semmle.label | call to method ToString : String | +| Test.cs:118:75:118:83 | access to local variable userInput | semmle.label | access to local variable userInput | +| Test.cs:119:75:119:83 | access to local variable userInput | semmle.label | access to local variable userInput | +| Test.cs:120:75:120:83 | access to local variable userInput | semmle.label | access to local variable userInput | +| Test.cs:122:76:122:84 | access to local variable userInput | semmle.label | access to local variable userInput | +| Test.cs:123:76:123:84 | access to local variable userInput | semmle.label | access to local variable userInput | +| Test.cs:124:76:124:84 | access to local variable userInput | semmle.label | access to local variable userInput | +| Test.cs:125:76:125:84 | access to local variable userInput | semmle.label | access to local variable userInput | +| Test.cs:126:76:126:84 | access to local variable userInput | semmle.label | access to local variable userInput | +| Test.cs:127:76:127:84 | access to local variable userInput | semmle.label | access to local variable userInput | +| Test.cs:128:76:128:84 | access to local variable userInput | semmle.label | access to local variable userInput | +| Test.cs:129:76:129:84 | access to local variable userInput | semmle.label | access to local variable userInput | +| Test.cs:130:76:130:84 | access to local variable userInput | semmle.label | access to local variable userInput | +| Test.cs:131:77:131:85 | access to local variable userInput | semmle.label | access to local variable userInput | +| Test.cs:132:77:132:85 | access to local variable userInput | semmle.label | access to local variable userInput | +subpaths diff --git a/csharp/ql/test/query-tests/Security Features/CWE-502/UnsafeTypeNameHandlingWithoutBinder/UnsafeTypeNameHandlingWithoutBinder.qlref b/csharp/ql/test/query-tests/Security Features/CWE-502/UnsafeTypeNameHandlingWithoutBinder/UnsafeTypeNameHandlingWithoutBinder.qlref new file mode 100644 index 000000000000..c36150b4e45c --- /dev/null +++ b/csharp/ql/test/query-tests/Security Features/CWE-502/UnsafeTypeNameHandlingWithoutBinder/UnsafeTypeNameHandlingWithoutBinder.qlref @@ -0,0 +1,3 @@ +query: Security Features/CWE-502/UnsafeTypeNameHandlingWithoutBinder.ql +postprocess: +- utils/test/InlineExpectationsTestQuery.ql \ No newline at end of file diff --git a/csharp/ql/test/query-tests/Security Features/CWE-502/UnsafeTypeNameHandlingWithoutBinder/options b/csharp/ql/test/query-tests/Security Features/CWE-502/UnsafeTypeNameHandlingWithoutBinder/options new file mode 100644 index 000000000000..e1b4d7c360e6 --- /dev/null +++ b/csharp/ql/test/query-tests/Security Features/CWE-502/UnsafeTypeNameHandlingWithoutBinder/options @@ -0,0 +1,4 @@ +semmle-extractor-options: /nostdlib /noconfig +semmle-extractor-options: --load-sources-from-project:${testdir}/../../../../resources/stubs/Newtonsoft.Json/13.0.4/Newtonsoft.Json.csproj +semmle-extractor-options: --load-sources-from-project:${testdir}/../../../../resources/stubs/_frameworks/Microsoft.NETCore.App/Microsoft.NETCore.App.csproj +semmle-extractor-options: --load-sources-from-project:${testdir}/../../../../resources/stubs/_frameworks/Microsoft.AspNetCore.App/Microsoft.AspNetCore.App.csproj