Skip to content

Commit 202a091

Browse files
raulgarciamsftraulgarciamsft
committed
IdentityModel_tokenvalidation
1 parent b609f1e commit 202a091

16 files changed

+700
-0
lines changed

‎csharp/ql/src/experimental/Security Features/JsonWebTokenHandler/JsonWebTokenHandlerLib.qll

Lines changed: 263 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,263 @@
1+
import csharp
2+
import DataFlow
3+
4+
/**
5+
* Abstract PropertyWrite for `TokenValidationParameters`.
6+
* Not really necessary anymore, but keeping it in case we want to extend the queries to check on other properties.
7+
*/
8+
abstract class TokenValidationParametersPropertyWrite extends PropertyWrite { }
9+
10+
/**
11+
* An access to a sensitive property for `TokenValidationParameters` that updates the underlying value.
12+
*/
13+
class TokenValidationParametersPropertyWriteToBypassSensitiveValidation extends TokenValidationParametersPropertyWrite {
14+
TokenValidationParametersPropertyWriteToBypassSensitiveValidation() {
15+
exists(Property p, Class c |
16+
c.hasQualifiedName("Microsoft.IdentityModel.Tokens.TokenValidationParameters")
17+
|
18+
p.getAnAccess() = this and
19+
c.getAProperty() = p and
20+
p.getName() in [
21+
"ValidateIssuer", "ValidateAudience", "ValidateLifetime", "RequireExpirationTime"
22+
]
23+
)
24+
}
25+
}
26+
27+
/**
28+
* Dataflow from a `false` value to an to a write sensitive property for `TokenValidationParameters`.
29+
*/
30+
class FalseValueFlowsToTokenValidationParametersPropertyWriteToBypassValidation extends TaintTracking::Configuration {
31+
FalseValueFlowsToTokenValidationParametersPropertyWriteToBypassValidation() {
32+
this = "FlowsToTokenValidationResultIsValidCall"
33+
}
34+
35+
override predicate isSource(DataFlow::Node source) {
36+
source.asExpr().(BoolLiteral).getValue() = "false"
37+
}
38+
39+
override predicate isSink(DataFlow::Node sink) {
40+
exists(TokenValidationParametersPropertyWrite pw, Assignment a | a.getLValue() = pw |
41+
sink.asExpr() = a.getRValue()
42+
)
43+
}
44+
}
45+
46+
/**
47+
* Holds if `assemblyName` is older than version `ver`
48+
*/
49+
bindingset[ver]
50+
predicate isAssemblyOlderVersion(string assemblyName, string ver) {
51+
exists(Assembly a |
52+
a.getName() = assemblyName and
53+
a.getVersion().isEarlierThan(ver)
54+
)
55+
}
56+
57+
/**
58+
* Method `ValidateToken` for `Microsoft.IdentityModel.JsonWebTokens.JsonWebTokenHandler` or other Token handler that shares the same behavior characteristics
59+
*/
60+
class JsonWebTokenHandlerValidateTokenMethod extends Method {
61+
JsonWebTokenHandlerValidateTokenMethod() {
62+
this.hasQualifiedName("Microsoft.IdentityModel.JsonWebTokens.JsonWebTokenHandler.ValidateToken") or
63+
this.hasQualifiedName("Microsoft.AzureAD.DeviceIdentification.Common.Tokens.JwtValidator.ValidateEncryptedToken")
64+
//// TODO: ValidateEncryptedToken has the same behavior than ValidateToken, but it will be changed in a future release
65+
//// The line below would allow to check if the ValidateEncryptedToken version used meets the minimum requirement
66+
//// Once we have the fixed assembly version we can uncomment the line below
67+
// and isAssemblyOlderVersion("Microsoft.AzureAD.DeviceIdentification", "0.0.0")
68+
}
69+
}
70+
71+
/**
72+
* A Call to `Microsoft.IdentityModel.JsonWebTokens.JsonWebTokenHandler.ValidateToken`
73+
*/
74+
class JsonWebTokenHandlerValidateTokenCall extends MethodCall {
75+
JsonWebTokenHandlerValidateTokenCall() {
76+
exists(JsonWebTokenHandlerValidateTokenMethod m | m.getACall() = this)
77+
}
78+
}
79+
80+
/**
81+
* Read access for properties `IsValid` or `Exception` for `Microsoft.IdentityModel.JsonWebTokens.JsonWebTokenHandler.ValidateToken`
82+
*/
83+
class TokenValidationResultIsValidCall extends PropertyRead {
84+
TokenValidationResultIsValidCall() {
85+
exists(Property p | p.getAnAccess().(PropertyRead) = this |
86+
p.hasName("IsValid") or
87+
p.hasName("Exception")
88+
)
89+
}
90+
}
91+
92+
/**
93+
* Dataflow from the output of `Microsoft.IdentityModel.JsonWebTokens.JsonWebTokenHandler.ValidateToken` call to access the `IsValid` or `Exception` property
94+
*/
95+
private class FlowsToTokenValidationResultIsValidCall extends TaintTracking::Configuration {
96+
FlowsToTokenValidationResultIsValidCall() { this = "FlowsToTokenValidationResultIsValidCall" }
97+
98+
override predicate isSource(DataFlow::Node source) {
99+
source.asExpr() instanceof JsonWebTokenHandlerValidateTokenCall
100+
}
101+
102+
override predicate isSink(DataFlow::Node sink) {
103+
exists(TokenValidationResultIsValidCall call | sink.asExpr() = call.getQualifier())
104+
}
105+
}
106+
107+
/**
108+
* Holds if the call to `Microsoft.IdentityModel.JsonWebTokens.JsonWebTokenHandler.ValidateToken` flows to any `IsValid` or `Exception` property access
109+
*/
110+
predicate hasAFlowToTokenValidationResultIsValidCall(JsonWebTokenHandlerValidateTokenCall call) {
111+
exists(FlowsToTokenValidationResultIsValidCall config, DataFlow::Node source |
112+
call = source.asExpr()
113+
|
114+
config.hasFlow(source, _)
115+
)
116+
}
117+
118+
/**
119+
* Property write for security-sensitive properties for `Microsoft.IdentityModel.Tokens.TokenValidationParameters`
120+
*/
121+
class TokenValidationParametersPropertyWriteToValidationDelegated extends PropertyWrite {
122+
TokenValidationParametersPropertyWriteToValidationDelegated() {
123+
exists(Property p, Class c |
124+
c.hasQualifiedName("Microsoft.IdentityModel.Tokens.TokenValidationParameters")
125+
|
126+
p.getAnAccess() = this and
127+
c.getAProperty() = p and
128+
p.getName() in [
129+
"SignatureValidator", "TokenReplayValidator", "AlgorithmValidator", "AudienceValidator",
130+
"IssuerSigningKeyValidator", "LifetimeValidator"
131+
]
132+
)
133+
}
134+
}
135+
136+
/**
137+
* Holds if the callable has a return statement and it always returns true for all such statements
138+
*/
139+
predicate callableHasARetrunStmtAndAlwaysReturnsTrue(Callable c) {
140+
c.getReturnType().toString() = "Boolean" and
141+
forall(ReturnStmt rs | rs.getEnclosingCallable() = c |
142+
rs.getChildExpr(0).(BoolLiteral).getBoolValue() = true
143+
) and
144+
exists(ReturnStmt rs | rs.getEnclosingCallable() = c)
145+
}
146+
147+
/**
148+
* Holds if the lambda expression `le` always returns true
149+
*/
150+
predicate lambdaExprReturnsOnlyLiteralTrue(LambdaExpr le) {
151+
le.getExpressionBody().(BoolLiteral).getBoolValue() = true
152+
}
153+
154+
class CallableAlwaysReturnsTrue extends Callable {
155+
CallableAlwaysReturnsTrue() {
156+
callableHasARetrunStmtAndAlwaysReturnsTrue(this)
157+
or
158+
lambdaExprReturnsOnlyLiteralTrue(this)
159+
or
160+
exists(LambdaExpr le, Call call, CallableAlwaysReturnsTrue cat | this = le |
161+
call = le.getExpressionBody() and
162+
cat.getACall() = call
163+
)
164+
}
165+
}
166+
167+
/**
168+
* Holds if any exception being thrown by the callable is of type `System.ArgumentNullException`
169+
* It will also hold if no exceptions are thrown by the callable
170+
*/
171+
predicate callableOnlyThrowsArgumentNullException(Callable c) {
172+
forall(ThrowElement thre | c = thre.getEnclosingCallable() |
173+
thre.getThrownExceptionType().hasQualifiedName("System.ArgumentNullException")
174+
)
175+
}
176+
177+
/**
178+
* A specialization of `CallableAlwaysReturnsTrue` that takes into consideration exceptions being thrown for higher precision.
179+
*/
180+
class CallableAlwaysReturnsTrueHigherPrecision extends CallableAlwaysReturnsTrue {
181+
CallableAlwaysReturnsTrueHigherPrecision() {
182+
callableOnlyThrowsArgumentNullException(this) and
183+
(
184+
forall(Call call, Callable callable | call.getEnclosingCallable() = this |
185+
callable.getACall() = call and
186+
callable instanceof CallableAlwaysReturnsTrueHigherPrecision
187+
)
188+
or
189+
exists(LambdaExpr le, Call call, CallableAlwaysReturnsTrueHigherPrecision cat | this = le |
190+
call = le.getExpressionBody() and
191+
cat.getACall() = call
192+
)
193+
)
194+
}
195+
}
196+
197+
/**
198+
* Property Write for the `IssuerValidator` property for `Microsoft.IdentityModel.Tokens.TokenValidationParameters`
199+
*/
200+
class TokenValidationParametersPropertyWriteToValidationDelegatedIssuerValidator extends PropertyWrite {
201+
TokenValidationParametersPropertyWriteToValidationDelegatedIssuerValidator() {
202+
exists(Property p, Class c |
203+
c.hasQualifiedName("Microsoft.IdentityModel.Tokens.TokenValidationParameters")
204+
|
205+
p.getAnAccess() = this and
206+
c.getAProperty() = p and
207+
p.getName() in ["IssuerValidator"]
208+
)
209+
}
210+
}
211+
212+
/**
213+
* A callable that returns a `string` and has a `string` as 1st argument
214+
*/
215+
private class CallableReturnsStringAndArg0IsString extends Callable {
216+
CallableReturnsStringAndArg0IsString() {
217+
this.getReturnType().toString() = "String" and
218+
this.getParameter(0).getType().toString() = "String"
219+
}
220+
}
221+
222+
/**
223+
* A Callable that always retrun the 1st argument, both of `string` type
224+
*/
225+
class CallableAlwatsReturnsParameter0 extends CallableReturnsStringAndArg0IsString {
226+
CallableAlwatsReturnsParameter0() {
227+
forall(ReturnStmt rs | rs.getEnclosingCallable() = this |
228+
rs.getChild(0) = this.getParameter(0).getAnAccess()
229+
) and
230+
exists(ReturnStmt rs | rs.getEnclosingCallable() = this)
231+
or
232+
exists(LambdaExpr le, Call call, CallableAlwatsReturnsParameter0 cat | this = le |
233+
call = le.getExpressionBody() and
234+
cat.getACall() = call
235+
)
236+
or
237+
this.getBody() = this.getParameter(0).getAnAccess()
238+
}
239+
}
240+
241+
/**
242+
* A Callable that always retrun the 1st argument, both of `string` type. Higher precision
243+
*/
244+
class CallableAlwatsReturnsParameter0MayThrowExceptions extends CallableReturnsStringAndArg0IsString {
245+
CallableAlwatsReturnsParameter0MayThrowExceptions() {
246+
callableOnlyThrowsArgumentNullException(this) and
247+
forall(ReturnStmt rs | rs.getEnclosingCallable() = this |
248+
rs.getChild(0) = this.getParameter(0).getAnAccess()
249+
) and
250+
exists(ReturnStmt rs | rs.getEnclosingCallable() = this)
251+
or
252+
exists(LambdaExpr le, Call call, CallableAlwatsReturnsParameter0MayThrowExceptions cat |
253+
this = le
254+
|
255+
call = le.getExpressionBody() and
256+
cat.getACall() = call and
257+
callableOnlyThrowsArgumentNullException(le) and
258+
callableOnlyThrowsArgumentNullException(cat)
259+
)
260+
or
261+
this.getBody() = this.getParameter(0).getAnAccess()
262+
}
263+
}

‎csharp/ql/src/experimental/Security Features/JsonWebTokenHandler/delegated-security-validations-always-return-true-bad.cs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
using System;
2+
using Microsoft.IdentityModel.Tokens;
3+
class TestClass
4+
{
5+
public void TestMethod()
6+
{
7+
TokenValidationParameters parameters = new TokenValidationParameters();
8+
parameters.AudienceValidator = (audiences, token, tvp) => { return true; };
9+
}
10+
}

‎csharp/ql/src/experimental/Security Features/JsonWebTokenHandler/delegated-security-validations-always-return-true-good.cs

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
using System;
2+
using Microsoft.IdentityModel.Tokens;
3+
class TestClass
4+
{
5+
public void TestMethod()
6+
{
7+
TokenValidationParameters parameters = new TokenValidationParameters();
8+
parameters.AudienceValidator = (audiences, token, tvp) =>
9+
{
10+
// Implement your own custom audience validation
11+
if (PerformCustomAudienceValidation(audiences, token))
12+
return true;
13+
else
14+
return false;
15+
};
16+
}
17+
}

‎csharp/ql/src/experimental/Security Features/JsonWebTokenHandler/delegated-security-validations-always-return-true.qhelp

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
<overview>
6+
<p>By setting critical <code>TokenValidationParameter</code> validation delegates to always return <code>true</code>, important authentication safeguards are disabled. Disabling safeguards can lead to incorrect validation of tokens from any issuer or expired tokens.</p>
7+
8+
</overview>
9+
<recommendation>
10+
<p>Improve the logic of the delegate so not all code paths return <code>true</code>, which effectively disables that type of validation; or throw <code>SecurityTokenInvalidAudienceException</code> or <code>SecurityTokenInvalidLifetimeException</code> in failure cases when you want to fail validation and have other cases pass by returning <code>true</code>.
11+
</p>
12+
</recommendation>
13+
14+
<example>
15+
<p>This example delegates <code>AudienceValidator</code> to a callable that always returns true.</p>
16+
<sample src="delegated-security-validations-always-return-true-bad.cs" />
17+
18+
<p>To fix it, use a callable that performs a validation, and fails when appropriate.</p>
19+
<sample src="delegated-security-validations-always-return-true-good.cs" />
20+
21+
</example>
22+
23+
<references>
24+
25+
<li><a href="https://aka.ms/wilson/tokenvalidation">azure-activedirectory-identitymodel-extensions-for-dotnet ValidatingTokens wiki</a></li>
26+
27+
</references>
28+
</qhelp>

‎csharp/ql/src/experimental/Security Features/JsonWebTokenHandler/delegated-security-validations-always-return-true.ql

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
/**
2+
* @name Delegated security sensitive validations for JsonWebTokenHandler always return true, medium precision
3+
* @description Security sensitive validations for `JsonWebTokenHandler` are being delegated to a function that seems to always return true.
4+
* Higher precision version checks for exception throws, so less false positives are expected.
5+
* @kind problem
6+
* @tags security
7+
* JsonWebTokenHandler
8+
* manual-verification-required
9+
* @id cs/JsonWebTokenHandler/delegated-security-validations-always-return-true
10+
* @problem.severity warning
11+
* @precision high
12+
*/
13+
14+
import csharp
15+
import DataFlow
16+
import JsonWebTokenHandlerLib
17+
18+
from
19+
TokenValidationParametersPropertyWriteToValidationDelegated tv, Assignment a,
20+
CallableAlwaysReturnsTrueHigherPrecision e
21+
where a.getLValue() = tv and a.getRValue().getAChild*() = e
22+
select tv,
23+
"JsonWebTokenHandler security-sensitive property $@ is being delegated to $@.",
24+
tv, tv.getTarget().toString(), e, "a callable that always returns \"true\""

‎csharp/ql/src/experimental/Security Features/JsonWebTokenHandler/security-validation-disabled-bad.cs

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
using System;
2+
using Microsoft.IdentityModel.Tokens;
3+
class TestClass
4+
{
5+
public void TestMethod()
6+
{
7+
TokenValidationParameters parameters = new TokenValidationParameters();
8+
parameters.RequireExpirationTime = false;
9+
parameters.ValidateAudience = false;
10+
parameters.ValidateIssuer = false;
11+
parameters.ValidateLifetime = false;
12+
}
13+
}

‎csharp/ql/src/experimental/Security Features/JsonWebTokenHandler/security-validation-disabled-good.cs

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
using System;
2+
using Microsoft.IdentityModel.Tokens;
3+
class TestClass
4+
{
5+
public void TestMethod()
6+
{
7+
TokenValidationParameters parameters = new TokenValidationParameters();
8+
parameters.RequireExpirationTime = true;
9+
parameters.ValidateAudience = true;
10+
parameters.ValidateIssuer = true;
11+
parameters.ValidateLifetime = true;
12+
}
13+
}

0 commit comments

Comments
 (0)