Skip to content

Commit 65fd69e

Browse files
committed
added queries + unit tests
1 parent 49336e3 commit 65fd69e

25 files changed

Lines changed: 666 additions & 0 deletions
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
6+
<overview>
7+
<p>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.</p>
8+
9+
<p> The <a href="https://www.newtonsoft.com/json/help/html/T_Newtonsoft_Json_TypeNameHandling.htm"> Newtonsoft.Json.TypeNameHandling </a> enumeration value of <code>Auto</code> or <code>All</code> is vulnerable when deserializing untrusted data.
10+
An attacker could modify the serialized data to include unexpected types to inject objects with malicious side effects.
11+
An attack against an insecure deserializer could, for example, execute commands on the underlying operating system, communicate over the network, or delete files.</p>
12+
</overview>
13+
14+
<recommendation>
15+
<p>Use the <a href="https://www.newtonsoft.com/json/help/html/T_Newtonsoft_Json_TypeNameHandling.htm">TypeNameHandling</a> value of <code>None</code>, or use a custom ISerializationBinder.</p>
16+
</recommendation>
17+
18+
<example>
19+
20+
<p>Violation:</p>
21+
22+
<sample src="examples/UnsafeTypeNameHandlingBad.cs" />
23+
24+
<p>Solution using TypeNameHandling.None:</p>
25+
26+
<sample src="examples/UnsafeTypeNameHandlingGood.cs" />
27+
28+
<p>Solution using custom ISerializationBinder: </p>
29+
30+
<sample src="examples/UnsafeTypeNameHandlingWithBinderGood.cs" />
31+
32+
</example>
33+
34+
<references>
35+
<li>Microsoft: <a href="https://docs.microsoft.com/en-us/dotnet/fundamentals/code-analysis/quality-rules/ca2326">Do Not Use TypeNameHandling Values Other Than None</a>.</li>
36+
</references>
37+
38+
</qhelp>
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
6+
<overview>
7+
<p> The <a href="https://www.newtonsoft.com/json/help/html/T_Newtonsoft_Json_TypeNameHandling.htm"> Newtonsoft.Json.TypeNameHandling </a> enumeration value of other than <code>None</code> is vulnerable when deserializing untrusted data.
8+
An attacker could modify the serialized data to include unexpected types to inject objects with malicious side effects.
9+
An attack against an insecure deserializer could, for example, execute commands on the underlying operating system, communicate over the network, or delete files.</p>
10+
11+
<p>This query detects assignment of unsafe <code>TypeNameHandling</code> values to <code>JsonConvert.DefaultSettings</code>, and where these settings are used to deserialize user input</p>
12+
13+
</overview>
14+
15+
<recommendation>
16+
<p>Use the <a href="https://www.newtonsoft.com/json/help/html/T_Newtonsoft_Json_TypeNameHandling.htm">TypeNameHandling</a> value of <code>None</code>, or use a custom ISerializationBinder.</p>
17+
</recommendation>
18+
19+
<example>
20+
21+
<p>Violation:</p>
22+
23+
<sample src="examples/UnsafeTypeNameHandlingBad.cs" />
24+
25+
<p>Solution using <code>TypeNameHandling.None</code>:</p>
26+
27+
<sample src="examples/UnsafeTypeNameHandlingGood.cs" />
28+
29+
<p>Solution using custom <code>ISerializationBinder</code>: </p>
30+
31+
<sample src="examples/UnsafeTypeNameHandlingWithBinderGood.cs" />
32+
33+
</example>
34+
35+
<references>
36+
<li>Microsoft: <a href="https://docs.microsoft.com/en-us/dotnet/fundamentals/code-analysis/quality-rules/ca2326">Do Not Use TypeNameHandling Values Other Than None</a>.</li>
37+
</references>
38+
39+
</qhelp>
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
6+
<overview>
7+
<p> The <a href="https://www.newtonsoft.com/json/help/html/T_Newtonsoft_Json_TypeNameHandling.htm"> Newtonsoft.Json.TypeNameHandling </a> enumeration value of other than <code>None</code> is vulnerable when deserializing untrusted data.
8+
An attacker could modify the serialized data to include unexpected types to inject objects with malicious side effects.
9+
An attack against an insecure deserializer could, for example, execute commands on the underlying operating system, communicate over the network, or delete files.</p>
10+
11+
<p>This query detects assignment of unsafe <code>TypeNameHandling</code> values in <code>Startup.cs</code> using <code>AddJsonOptions</code> or <code>AddNewtonsoftJson</code>, and where these settings are used to deserialize user input</p>
12+
13+
</overview>
14+
15+
<recommendation>
16+
<p>Use the <a href="https://www.newtonsoft.com/json/help/html/T_Newtonsoft_Json_TypeNameHandling.htm">TypeNameHandling</a> value of <code>None</code>, or use a custom ISerializationBinder.</p>
17+
</recommendation>
18+
19+
<example>
20+
21+
<p>Violation:</p>
22+
23+
<sample src="examples/UnsafeTypeNameHandlingBad.cs" />
24+
25+
<p>Solution using <code>TypeNameHandling.None</code>:</p>
26+
27+
<sample src="examples/UnsafeTypeNameHandlingGood.cs" />
28+
29+
<p>Solution using custom <code>ISerializationBinder</code>: </p>
30+
31+
<sample src="examples/UnsafeTypeNameHandlingWithBinderGood.cs" />
32+
33+
</example>
34+
35+
<references>
36+
<li>Microsoft: <a href="https://docs.microsoft.com/en-us/dotnet/fundamentals/code-analysis/quality-rules/ca2326">Do Not Use TypeNameHandling Values Other Than None</a>.</li>
37+
</references>
38+
39+
</qhelp>
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
6+
<overview>
7+
<p>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.</p>
8+
9+
10+
<p> The <a href="https://www.newtonsoft.com/json/help/html/T_Newtonsoft_Json_TypeNameHandling.htm"> Newtonsoft.Json.TypeNameHandling </a> enumeration value of <code>Auto</code> or <code>All</code> is vulnerable when deserializing untrusted data.
11+
An attacker could modify the serialized data to include unexpected types to inject objects with malicious side effects.
12+
An attack against an insecure deserializer could, for example, execute commands on the underlying operating system, communicate over the network, or delete files.</p>
13+
</overview>
14+
15+
<recommendation>
16+
<p>Use the <a href="https://www.newtonsoft.com/json/help/html/T_Newtonsoft_Json_TypeNameHandling.htm">TypeNameHandling</a> value of <code>None</code>, or use a custom ISerializationBinder.</p>
17+
</recommendation>
18+
19+
<example>
20+
21+
<p>Violation:</p>
22+
23+
<sample src="examples/UnsafeTypeNameHandlingBad.cs" />
24+
25+
<p>Solution using TypeNameHandling.None:</p>
26+
27+
<sample src="examples/UnsafeTypeNameHandlingGood.cs" />
28+
29+
<p>Solution using custom ISerializationBinder: </p>
30+
31+
<sample src="examples/UnsafeTypeNameHandlingWithBinderGood.cs" />
32+
33+
</example>
34+
35+
<references>
36+
<li>Microsoft: <a href="https://docs.microsoft.com/en-us/dotnet/fundamentals/code-analysis/quality-rules/ca2326">Do Not Use TypeNameHandling Values Other Than None</a>.</li>
37+
</references>
38+
39+
</qhelp>

csharp/ql/src/Security Features/CWE-502/UnsafeTypeNameHandlingWithoutBinder.ql

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,8 @@ module TypeNameFlowConfig implements DataFlow::ConfigSig {
9191

9292
module TypeNameFlow = DataFlow::Global<TypeNameFlowConfig>;
9393

94+
import UserInputToDeserializeObjectCallFlow::PathGraph
95+
9496
from
9597
UserInputToDeserializeObjectCallFlow::PathNode userInput, UserInputToDeserializeObjectCallFlow::PathNode deserializeArg,
9698
DataFlow::Node badTypeNameHandling, DataFlow::Node typeNameHandlingSettingArg,
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
using Newtonsoft.Json;
2+
public class ExampleClass
3+
{
4+
public JsonSerializerSettings Settings { get; }
5+
public ExampleClass()
6+
{
7+
Settings = new JsonSerializerSettings();
8+
Settings.TypeNameHandling = TypeNameHandling.All; // BAD
9+
}
10+
}
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
using Newtonsoft.Json;
2+
public class ExampleClass
3+
{
4+
public JsonSerializerSettings Settings { get; }
5+
public ExampleClass()
6+
{
7+
Settings = new JsonSerializerSettings(); // GOOD: The default value of Settings.TypeNameHandling is TypeNameHandling.None.
8+
}
9+
}
Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
using Newtonsoft.Json;
2+
using Newtonsoft.Json.Serialization;
3+
using System;
4+
using System.Collections.Generic;
5+
using System.Linq;
6+
using System.Text;
7+
using System.Threading.Tasks;
8+
9+
namespace TypeHandlingSample
10+
{
11+
public class KnownTypesBinder : ISerializationBinder
12+
{
13+
public IList<Type> KnownTypes { get; set; }
14+
15+
public Type BindToType(string assemblyName, string typeName)
16+
{
17+
return KnownTypes.SingleOrDefault(t => t.Name == typeName);
18+
}
19+
20+
public void BindToName(Type serializedType, out string assemblyName, out string typeName)
21+
{
22+
assemblyName = null;
23+
typeName = serializedType.Name;
24+
}
25+
}
26+
public class SomeClass
27+
{
28+
public string Name { get; set; }
29+
}
30+
31+
internal class Okay
32+
{
33+
public JsonSerializerSettings Settings { get; }
34+
public KnownTypesBinder Binder { get; }
35+
public Okay()
36+
{
37+
Settings = new JsonSerializerSettings();
38+
Binder = new KnownTypesBinder { KnownTypes = new List<Type> { typeof(SomeClass) } };
39+
Settings.SerializationBinder = Binder;
40+
Settings.TypeNameHandling = TypeNameHandling.All; // Okay, because SerializationBinder set to custom ISerializationBinder
41+
}
42+
}
43+
}
Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
1+
using System;
2+
using System.Web;
3+
using System.Text;
4+
using System.Collections.Generic;
5+
using Newtonsoft.Json;
6+
using Newtonsoft.Json.Serialization;
7+
using System.Threading.Tasks;
8+
9+
namespace TypeHandlingSample
10+
{
11+
public class BadBinder : ISerializationBinder
12+
{
13+
public void BindToName(Type serializedType, out string assemblyName, out string typeName)
14+
{
15+
assemblyName = serializedType.Assembly.FullName;
16+
typeName = serializedType.Name;
17+
}
18+
19+
public Type BindToType(string assemblyName, string typeName) // $ Alert
20+
{
21+
return Type.GetType(typeName);
22+
}
23+
}
24+
25+
public class OkayBinder : DefaultSerializationBinder
26+
{
27+
public List<string> okayTypes;
28+
public override Type BindToType(string? assemblyName, string typeName) // $ Alert
29+
{
30+
if (okayTypes.Contains(typeName))
31+
{
32+
return Type.GetType(typeName);
33+
34+
}
35+
else
36+
{
37+
throw new Exception("unexpected type");
38+
}
39+
}
40+
}
41+
42+
public class OkayBinder2 : ISerializationBinder
43+
{
44+
private readonly ISerializationBinder innerBinder;
45+
46+
public void BindToName(Type serializedType, out string? assemblyName, out string? typeName)
47+
{
48+
this.innerBinder.BindToName(serializedType, out assemblyName, out typeName);
49+
}
50+
public Type BindToType(string? assemblyName, string typeName) // $ Alert
51+
{
52+
return this.innerBinder.BindToType(assemblyName, typeName);
53+
}
54+
}
55+
56+
public class Test
57+
{
58+
public JsonSerializerSettings SettingsBad { get; }
59+
60+
public JsonSerializerSettings SettingsOkay { get; }
61+
62+
public JsonSerializerSettings SettingsOkay2 { get; }
63+
64+
public BadBinder badBinder { get; }
65+
66+
public OkayBinder okayBinder { get; }
67+
68+
public OkayBinder2 okayBinder2 { get; }
69+
public Test()
70+
{
71+
//Bad, custom binder that does not check type
72+
SettingsBad = new JsonSerializerSettings()
73+
{
74+
SerializationBinder = badBinder,
75+
TypeNameHandling = TypeNameHandling.All
76+
};
77+
78+
//OKAY, custom binder that checks type
79+
SettingsOkay = new JsonSerializerSettings()
80+
{
81+
SerializationBinder = okayBinder,
82+
TypeNameHandling = TypeNameHandling.All
83+
};
84+
85+
//OKAY, custom binder that returns BindToType() from a member binder
86+
SettingsOkay2 = new JsonSerializerSettings()
87+
{
88+
SerializationBinder = okayBinder2,
89+
TypeNameHandling = TypeNameHandling.All
90+
};
91+
}
92+
}
93+
94+
}
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
| 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 |
2+
| 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 |
3+
| 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 |

0 commit comments

Comments
 (0)