Skip to content

Commit 3759158

Browse files
ropwareJBropwareJB
authored
Merge pull request #284 from microsoft/jb1/ab13144-migration
Migrate WilsonLib directory to public repo
2 parents aa81060 + 97f11a1 commit 3759158

File tree

188 files changed

+69028
-0
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

188 files changed

+69028
-0
lines changed

‎csharp/ql/src/Security Features/JWT/WilsonLib/AadIssuerValidationDataFlow.qhelp‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
<overview>
6+
<p>Must validate the AAD key issuer if you use IdentityModel directly to validate Azure AD tokens. See the link below for details</p>
7+
</overview>
8+
<references>
9+
10+
<li><a href="https://aka.ms/wilson/vul-23030">Validate the AAD key issuer if you use IdentityModel directly to validate Azure AD tokens wiki</a></li>
11+
12+
</references>
13+
</qhelp>

‎csharp/ql/src/Security Features/JWT/WilsonLib/AadIssuerValidationDataFlow.ql‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
/**
2+
* @name AAD Issuer Validation With Data Flow
3+
* @description Verify that after creating a new `TokenValidationParameters` object, there is a dataflow to call `EnableAadSigningKeyIssuerValidation`.
4+
* @kind problem
5+
* @tags security
6+
* wilson-library
7+
* @id cs/wilson-library/aad-issuer-validation-data-flow
8+
* @problem.severity error
9+
*/
10+
11+
import csharp
12+
import WilsonLibAad
13+
14+
from TokenValidationParametersObjectCreation oc
15+
where
16+
not isTokenValidationParametersCallingEnableAadSigningKeyIssuerValidation(oc) and
17+
oc.fromSource()
18+
select oc,
19+
"The usage of Wilson library without validating the AAD key issuer if you use IdentityModel directly to validate Azure AD tokens was detected. Visit https://aka.ms/wilson/vul-23030 for details."

‎csharp/ql/src/Security Features/JWT/WilsonLib/ExistsAadIssuerValidationCall.qhelp‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
<overview>
6+
<p>Must validate the AAD key issuer if you use IdentityModel directly to validate Azure AD tokens. See the link below for details</p>
7+
</overview>
8+
<references>
9+
10+
<li><a href="https://aka.ms/wilson/vul-23030">Validate the AAD key issuer if you use IdentityModel directly to validate Azure AD tokens wiki</a></li>
11+
12+
</references>
13+
</qhelp>

‎csharp/ql/src/Security Features/JWT/WilsonLib/ExistsAadIssuerValidationCall.ql‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
/**
2+
* @name Exists AAD Issuer Validation Call
3+
* @description Verify that there is at least one call to `EnableAadSigningKeyIssuerValidation` if we detect the usage of Wilson library.
4+
* @kind problem
5+
* @tags security
6+
* wilson-library
7+
* @id cs/wilson-library/exists-aad-issuer-validation-call
8+
* @problem.severity error
9+
*/
10+
11+
import csharp
12+
import WilsonLibAad
13+
14+
from TokenValidationParametersObjectCreation oc
15+
where
16+
not exists(MethodCall c | c instanceof EnableAadSigningKeyIssuerValidationMethodCall) and
17+
oc.fromSource()
18+
select oc,
19+
"A call to Wilson library's $@ without any call to `EnableAadSigningKeyIssuerValidation`. Visit https://aka.ms/wilson/vul-23030 for details.",
20+
oc, oc.getTarget().toString()

‎csharp/ql/src/Security Features/JWT/WilsonLib/WilsonLibAad.qll‎

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
import csharp
2+
import DataFlow
3+
4+
private module TokenValidationParametersFlowsToAadTokenValidationParametersExtensionCallConfiguration
5+
implements DataFlow::ConfigSig
6+
{
7+
predicate isSource(DataFlow::Node source) {
8+
exists(TokenValidationParametersObjectCreation oc | source.asExpr() = oc)
9+
}
10+
11+
predicate isSink(DataFlow::Node sink) {
12+
isExprEnableAadSigningKeyIssuerValidationMethodCall(sink.asExpr())
13+
}
14+
15+
/***
16+
* Additional steps needed because the setter i not `fromSource`
17+
*
18+
* We should eventually add this type of additional steps to the general `DataFlow` library we use
19+
*/
20+
predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
21+
exists(AssignExpr ae, Expr oc
22+
|
23+
node1.asExpr() = oc and
24+
node2.asExpr() = ae.getLValue()
25+
|
26+
ae = oc.getParent()
27+
)
28+
or
29+
exists(PropertyAccess e2, PropertyWrite e1
30+
|
31+
node1.asExpr() = e1 and
32+
node2.asExpr() = e2
33+
|
34+
e1.getAControlFlowNode().dominates(e2.getAControlFlowNode()) and
35+
e2.getTarget().getSetter().getACall() = e1
36+
)
37+
}
38+
39+
40+
additional predicate isExprEnableAadSigningKeyIssuerValidationMethodCall(Expr e) {
41+
exists(EnableAadSigningKeyIssuerValidationMethodCall c, PropertyAccess pa | e = pa |
42+
c.getAChild() = pa
43+
)
44+
or
45+
exists(EnableAadSigningKeyIssuerValidationMethodCall c, VariableAccess va | e = va |
46+
c.getAChild() = va
47+
)
48+
}
49+
}
50+
51+
private module TokenValidationParametersFlowsToAadTokenValidationParametersExtensionCallTT =
52+
TaintTracking::Global<TokenValidationParametersFlowsToAadTokenValidationParametersExtensionCallConfiguration>;
53+
54+
class TokenValidationParametersObjectCreation extends ObjectCreation {
55+
TokenValidationParametersObjectCreation() {
56+
this.getObjectType()
57+
.hasFullyQualifiedName("Microsoft.IdentityModel.Tokens", "TokenValidationParameters")
58+
}
59+
}
60+
61+
class EnableAadSigningKeyIssuerValidationMethodCall extends MethodCall {
62+
EnableAadSigningKeyIssuerValidationMethodCall() {
63+
this.getTarget()
64+
.hasFullyQualifiedName("Microsoft.IdentityModel.Validators.AadTokenValidationParametersExtension",
65+
"EnableAadSigningKeyIssuerValidation")
66+
}
67+
}
68+
69+
predicate isTokenValidationParametersCallingEnableAadSigningKeyIssuerValidation(
70+
TokenValidationParametersObjectCreation oc
71+
) {
72+
exists(DataFlow::Node source, DataFlow::Node sink | oc = source.asExpr() |
73+
TokenValidationParametersFlowsToAadTokenValidationParametersExtensionCallTT::flow(source, sink)
74+
)
75+
}

‎csharp/ql/src/Security Features/JWT/WilsonLib/check-isvalid.qhelp‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
<overview>
6+
<p><code>JsonWebTokenHandler.ValidateToken</code> returns a <code>TokenValidationResult</code> object but does not throw an exception when token validation fails. </p>
7+
8+
<p>Instead, developers must explicitly check the <code>IsValid</code> property or the <code>Exception</code> property of the <code>TokenValidationResult</code>.
9+
If neither check is performed, the application may treat an invalid token as valid, leading to security vulnerabilities such as unauthorized access.</p>
10+
</overview>
11+
12+
<recommendation>
13+
<p>Always verify the <code>TokenValidationResult</code> returned by <code>ValidateToken</code> by checking the <code>IsValid</code> property or <code>Exception</code> property. </p>
14+
</recommendation>
15+
16+
<references>
17+
18+
<li><a href="https://learn.microsoft.com/en-us/dotnet/api/microsoft.identitymodel.jsonwebtokens.jsonwebtokenhandler.validatetoken">JsonWebTokenHandler.ValidateToken</a></li>
19+
20+
</references>
21+
</qhelp>

‎csharp/ql/src/Security Features/JWT/WilsonLib/check-isvalid.ql‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
/**
2+
* @name JsonWebTokenHandler.ValidateToken return value is not checked
3+
* @description `JsonWebTokenHandler.ValidateToken` does not throw an exception if the return value is not valid.
4+
* This query checks if the returning object `TokenValidationResult` is not accessing either the `IsValid` property or is accessing `Exception` property.
5+
* @kind problem
6+
* @tags security
7+
* wilson-library
8+
* manual-verification-required
9+
* @id cs/wilson-library/check-isvalid
10+
* @problem.severity warning
11+
* @precision high
12+
*/
13+
14+
import csharp
15+
import wilsonlib
16+
17+
from JsonWebTokenHandlerValidateTokenCall call
18+
where
19+
not hasAFlowToTokenValidationResultIsValidCall(call) and
20+
not call.getEnclosingCallable() instanceof JsonWebTokenHandlerValidateTokenMethod
21+
select call,
22+
"The call to $@ does not throw an exception if the resulting `TokenValidationResult` object is not valid, and the code in $@ is not checking the `IsValid` property or the `Exception` property to verify.",
23+
call, "`JsonWebTokenHandler.ValidateToken`", call.getEnclosingCallable(),
24+
getFullyQualifiedName(call.getEnclosingCallable())

‎csharp/ql/src/Security Features/JWT/WilsonLib/delegated-security-validations-always-permit-signature.qhelp‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
6+
<overview>
7+
<p>By setting <code>TokenValidationParameter.SignatureValidator</code> validation delegate to a callable that never throws an <code>Exception</code>, validation of the token signature is disabled. Disabling safeguards can lead to security compromise of tokens from any issuer.</p>
8+
</overview>
9+
10+
<recommendation>
11+
<p>Improve the logic of the delegate so to throw an <code>SecurityTokenException</code> in failure cases when you want to fail validation.</p>
12+
</recommendation>
13+
14+
<example>
15+
<p>This example delegates <code>SignatureValidator</code> to a callable that always returns a <code>Microsoft.IdentityModel.Tokens.SecurityToken</code>.</p>
16+
<sample src="examples/delegated-security-validations-always-permit-signature-bad.cs" />
17+
<p>To fix it, use a callable that performs a validation, and fails when appropriate.</p>
18+
<sample src="examples/delegated-security-validations-always-permit-signature-good.cs" />
19+
</example>
20+
21+
<references>
22+
<li><a href="https://aka.ms/wilson/tokenvalidation">azure-activedirectory-identitymodel-extensions-for-dotnet ValidatingTokens wiki</a></li>
23+
</references>
24+
25+
</qhelp>

‎csharp/ql/src/Security Features/JWT/WilsonLib/delegated-security-validations-always-permit-signature.ql‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
/**
2+
* @name Delegated SignatureValidator for JsonWebTokenHandler never throws an exception
3+
* @description `SignatureValidator` delegator for `JsonWebTokenHandler` always return a `SecurityToken` back without any checks.
4+
* Medium precision version that does not check for subcall exception throws, so false positives are expected.
5+
* @kind problem
6+
* @tags security
7+
* wilson-library
8+
* manual-verification-required
9+
* @id cs/wilson-library/delegated-security-validations-always-permit-signature
10+
* @problem.severity warning
11+
* @precision medium
12+
*/
13+
14+
import csharp
15+
import DataFlow
16+
import wilsonlib
17+
18+
module CallableReferenceFlowConfig implements DataFlow::ConfigSig{
19+
predicate isSource(DataFlow::Node source) {
20+
source.asExpr() instanceof Callable or
21+
source.asExpr() instanceof CallableAccess
22+
}
23+
24+
predicate isSink(DataFlow::Node sink) {
25+
exists(Assignment a | sink.asExpr() = a.getRValue())
26+
}
27+
28+
predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2){
29+
node1.asExpr() instanceof CallableAccess and
30+
node2.asExpr().(DelegateCreation).getArgument() = node1.asExpr()
31+
}
32+
}
33+
module CallableReferenceFlow = DataFlow::Global<CallableReferenceFlowConfig>;
34+
35+
from
36+
Assignment a,
37+
TokenValidationParametersPropertyWriteToValidationDelegatedSignatureValidator sv,
38+
Callable c,
39+
DataFlow::Node callableSource
40+
where
41+
a.getLValue() = sv and
42+
not callableThrowsException(c) and
43+
CallableReferenceFlow::flow(callableSource, DataFlow::exprNode(a.getRValue())) and
44+
(callableSource = DataFlow::exprNode(c) or callableSource.asExpr().(CallableAccess).getTarget() = c)
45+
select
46+
sv,
47+
"Delegated $@ for `JsonWebTokenHandler` is $@ that always returns a SecurityToken without any check that throws a SecurityTokenException.",
48+
sv, "SignatureValidator",
49+
c, "a callable"
50+

‎csharp/ql/src/Security Features/JWT/WilsonLib/delegated-security-validations-always-return-issuer-parameter.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 <code>TokenValidationParameter.IssuerValidator</code> validation delegate to a callable that always return the <code>issuer</code> argument (1st argument), important authentication safeguards are disabled. Disabling safeguards can lead to incorrect validation of tokens from any issuer.</p>
7+
8+
</overview>
9+
<recommendation>
10+
<p>Improve the logic of the delegate so not all code paths return <code>issuer</code>, which effectively disables that type of validation; or throw <code>SecurityTokenException</code> in failure cases when you want to fail validation.
11+
</p>
12+
</recommendation>
13+
14+
<example>
15+
<p>This example delegates <code>IssuerValidator</code> to a callable that always returns the <code>issuer</code>.</p>
16+
<sample src="examples/delegated-security-validations-always-return-issuer-parameter-bad.cs" />
17+
18+
<p>To fix it, use a callable that performs a validation, and fails when appropriate.</p>
19+
<sample src="examples/delegated-security-validations-always-return-issuer-parameter-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>

0 commit comments

Comments
 (0)