Skip to content

Commit d958c04

Browse files
tamasvajktamasvajk
authored
Merge pull request #9693 from raulgarciamsft/Token_validation
Token validation
2 parents c0762df + 0125ecf commit d958c04

16 files changed

+721
-0
lines changed

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

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

‎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: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
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/json-webtoken-handler/delegated-security-validations-always-return-true
10+
* @problem.severity error
11+
* @precision high
12+
*/
13+
14+
import csharp
15+
import DataFlow
16+
import JsonWebTokenHandlerLib
17+
18+
from TokenValidationParametersProperty p, CallableAlwaysReturnsTrueHigherPrecision e
19+
where e = p.getAnAssignedValue()
20+
select e, "JsonWebTokenHandler security-sensitive property $@ is being delegated to $@.", p,
21+
p.getQualifiedName().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)