Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
<!DOCTYPE qhelp PUBLIC
"-//Semmle//qhelp//EN"
"qhelp.dtd">
<qhelp>
<overview>
<p>Must validate the AAD key issuer if you use IdentityModel directly to validate Azure AD tokens. See the link below for details</p>
</overview>
<references>

<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>

</references>
</qhelp>
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
/**
* @name AAD Issuer Validation With Data Flow
* @description Verify that after creating a new `TokenValidationParameters` object, there is a dataflow to call `EnableAadSigningKeyIssuerValidation`.
* @kind problem
* @tags security
* wilson-library
* @id cs/wilson-library/aad-issuer-validation-data-flow
* @problem.severity error
*/

import csharp
import WilsonLibAad

from TokenValidationParametersObjectCreation oc
where
not isTokenValidationParametersCallingEnableAadSigningKeyIssuerValidation(oc) and
oc.fromSource()
select oc,
"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."
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
<!DOCTYPE qhelp PUBLIC
"-//Semmle//qhelp//EN"
"qhelp.dtd">
<qhelp>
<overview>
<p>Must validate the AAD key issuer if you use IdentityModel directly to validate Azure AD tokens. See the link below for details</p>
</overview>
<references>

<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>

</references>
</qhelp>
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
/**
* @name Exists AAD Issuer Validation Call
* @description Verify that there is at least one call to `EnableAadSigningKeyIssuerValidation` if we detect the usage of Wilson library.
* @kind problem
* @tags security
* wilson-library
* @id cs/wilson-library/exists-aad-issuer-validation-call
* @problem.severity error
*/

import csharp
import WilsonLibAad

from TokenValidationParametersObjectCreation oc
where
not exists(MethodCall c | c instanceof EnableAadSigningKeyIssuerValidationMethodCall) and
oc.fromSource()
select oc,
"A call to Wilson library's $@ without any call to `EnableAadSigningKeyIssuerValidation`. Visit https://aka.ms/wilson/vul-23030 for details.",
oc, oc.getTarget().toString()
75 changes: 75 additions & 0 deletions csharp/ql/src/Security Features/JWT/WilsonLib/WilsonLibAad.qll
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
import csharp
import DataFlow

private module TokenValidationParametersFlowsToAadTokenValidationParametersExtensionCallConfiguration
implements DataFlow::ConfigSig
{
predicate isSource(DataFlow::Node source) {
exists(TokenValidationParametersObjectCreation oc | source.asExpr() = oc)
}

predicate isSink(DataFlow::Node sink) {
isExprEnableAadSigningKeyIssuerValidationMethodCall(sink.asExpr())
}

/***
* Additional steps needed because the setter i not `fromSource`
*
* We should eventually add this type of additional steps to the general `DataFlow` library we use
*/
predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
exists(AssignExpr ae, Expr oc
|
node1.asExpr() = oc and
node2.asExpr() = ae.getLValue()
|
ae = oc.getParent()
)
or
exists(PropertyAccess e2, PropertyWrite e1
|
node1.asExpr() = e1 and
node2.asExpr() = e2
|
e1.getAControlFlowNode().dominates(e2.getAControlFlowNode()) and
e2.getTarget().getSetter().getACall() = e1
)
}


additional predicate isExprEnableAadSigningKeyIssuerValidationMethodCall(Expr e) {
exists(EnableAadSigningKeyIssuerValidationMethodCall c, PropertyAccess pa | e = pa |
c.getAChild() = pa
)
or
exists(EnableAadSigningKeyIssuerValidationMethodCall c, VariableAccess va | e = va |
c.getAChild() = va
)
}
}

private module TokenValidationParametersFlowsToAadTokenValidationParametersExtensionCallTT =
TaintTracking::Global<TokenValidationParametersFlowsToAadTokenValidationParametersExtensionCallConfiguration>;

class TokenValidationParametersObjectCreation extends ObjectCreation {
TokenValidationParametersObjectCreation() {
this.getObjectType()
.hasFullyQualifiedName("Microsoft.IdentityModel.Tokens", "TokenValidationParameters")
}
}

class EnableAadSigningKeyIssuerValidationMethodCall extends MethodCall {
EnableAadSigningKeyIssuerValidationMethodCall() {
this.getTarget()
.hasFullyQualifiedName("Microsoft.IdentityModel.Validators.AadTokenValidationParametersExtension",
"EnableAadSigningKeyIssuerValidation")
}
}

predicate isTokenValidationParametersCallingEnableAadSigningKeyIssuerValidation(
TokenValidationParametersObjectCreation oc
) {
exists(DataFlow::Node source, DataFlow::Node sink | oc = source.asExpr() |
TokenValidationParametersFlowsToAadTokenValidationParametersExtensionCallTT::flow(source, sink)
)
}
21 changes: 21 additions & 0 deletions csharp/ql/src/Security Features/JWT/WilsonLib/check-isvalid.qhelp
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
<!DOCTYPE qhelp PUBLIC
"-//Semmle//qhelp//EN"
"qhelp.dtd">
<qhelp>
<overview>
<p><code>JsonWebTokenHandler.ValidateToken</code> returns a <code>TokenValidationResult</code> object but does not throw an exception when token validation fails. </p>

<p>Instead, developers must explicitly check the <code>IsValid</code> property or the <code>Exception</code> property of the <code>TokenValidationResult</code>.
If neither check is performed, the application may treat an invalid token as valid, leading to security vulnerabilities such as unauthorized access.</p>
</overview>

<recommendation>
<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>
</recommendation>

<references>

<li><a href="https://learn.microsoft.com/en-us/dotnet/api/microsoft.identitymodel.jsonwebtokens.jsonwebtokenhandler.validatetoken">JsonWebTokenHandler.ValidateToken</a></li>

</references>
</qhelp>
24 changes: 24 additions & 0 deletions csharp/ql/src/Security Features/JWT/WilsonLib/check-isvalid.ql
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
/**
* @name JsonWebTokenHandler.ValidateToken return value is not checked
* @description `JsonWebTokenHandler.ValidateToken` does not throw an exception if the return value is not valid.
* This query checks if the returning object `TokenValidationResult` is not accessing either the `IsValid` property or is accessing `Exception` property.
* @kind problem
* @tags security
* wilson-library
* manual-verification-required
* @id cs/wilson-library/check-isvalid
* @problem.severity warning
* @precision high
*/

import csharp
import wilsonlib

from JsonWebTokenHandlerValidateTokenCall call
where
not hasAFlowToTokenValidationResultIsValidCall(call) and
not call.getEnclosingCallable() instanceof JsonWebTokenHandlerValidateTokenMethod
select call,
"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.",
call, "`JsonWebTokenHandler.ValidateToken`", call.getEnclosingCallable(),
getFullyQualifiedName(call.getEnclosingCallable())
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
<!DOCTYPE qhelp PUBLIC
"-//Semmle//qhelp//EN"
"qhelp.dtd">
<qhelp>

<overview>
<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>
</overview>

<recommendation>
<p>Improve the logic of the delegate so to throw an <code>SecurityTokenException</code> in failure cases when you want to fail validation.</p>
</recommendation>

<example>
<p>This example delegates <code>SignatureValidator</code> to a callable that always returns a <code>Microsoft.IdentityModel.Tokens.SecurityToken</code>.</p>
<sample src="examples/delegated-security-validations-always-permit-signature-bad.cs" />
<p>To fix it, use a callable that performs a validation, and fails when appropriate.</p>
<sample src="examples/delegated-security-validations-always-permit-signature-good.cs" />
</example>

<references>
<li><a href="https://aka.ms/wilson/tokenvalidation">azure-activedirectory-identitymodel-extensions-for-dotnet ValidatingTokens wiki</a></li>
</references>

</qhelp>
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
/**
* @name Delegated SignatureValidator for JsonWebTokenHandler never throws an exception
* @description `SignatureValidator` delegator for `JsonWebTokenHandler` always return a `SecurityToken` back without any checks.
* Medium precision version that does not check for subcall exception throws, so false positives are expected.
* @kind problem
* @tags security
* wilson-library
* manual-verification-required
* @id cs/wilson-library/delegated-security-validations-always-permit-signature
* @problem.severity warning
* @precision medium
*/

import csharp
import DataFlow
import wilsonlib

module CallableReferenceFlowConfig implements DataFlow::ConfigSig{
predicate isSource(DataFlow::Node source) {
source.asExpr() instanceof Callable or
source.asExpr() instanceof CallableAccess
}

predicate isSink(DataFlow::Node sink) {
exists(Assignment a | sink.asExpr() = a.getRValue())
}

predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2){
node1.asExpr() instanceof CallableAccess and
node2.asExpr().(DelegateCreation).getArgument() = node1.asExpr()
}
}
module CallableReferenceFlow = DataFlow::Global<CallableReferenceFlowConfig>;

from
Assignment a,
TokenValidationParametersPropertyWriteToValidationDelegatedSignatureValidator sv,
Callable c,
DataFlow::Node callableSource
where
a.getLValue() = sv and
not callableThrowsException(c) and
CallableReferenceFlow::flow(callableSource, DataFlow::exprNode(a.getRValue())) and
(callableSource = DataFlow::exprNode(c) or callableSource.asExpr().(CallableAccess).getTarget() = c)
select
sv,
"Delegated $@ for `JsonWebTokenHandler` is $@ that always returns a SecurityToken without any check that throws a SecurityTokenException.",
sv, "SignatureValidator",
c, "a callable"

Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
<!DOCTYPE qhelp PUBLIC
"-//Semmle//qhelp//EN"
"qhelp.dtd">
<qhelp>
<overview>
<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>

</overview>
<recommendation>
<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.
</p>
</recommendation>

<example>
<p>This example delegates <code>IssuerValidator</code> to a callable that always returns the <code>issuer</code>.</p>
<sample src="examples/delegated-security-validations-always-return-issuer-parameter-bad.cs" />

<p>To fix it, use a callable that performs a validation, and fails when appropriate.</p>
<sample src="examples/delegated-security-validations-always-return-issuer-parameter-good.cs" />

</example>

<references>

<li><a href="https://aka.ms/wilson/tokenvalidation">azure-activedirectory-identitymodel-extensions-for-dotnet ValidatingTokens wiki</a></li>

</references>
</qhelp>
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
/**
* @name Delegated IssuerValidator for JsonWebTokenHandler always return issuer argument back
* @description `IssuerValidator` delegator for `JsonWebTokenHandler` always return the `issuer` argument back without any checks.
* Medium precision version that does not check for exception throws, so false positives are expected.
* @kind problem
* @tags security
* wilson-library
* manual-verification-required
* @id cs/wilson-library/delegated-security-validations-always-return-issuer-parameter
* @problem.severity warning
* @precision medium
*/

import csharp
import DataFlow
import wilsonlib

from
TokenValidationParametersPropertyWriteToValidationDelegatedIssuerValidator tv, Assignment a,
CallableAlwaysReturnsParameter0MayThrowExceptions e
where a.getLValue() = tv and a.getRValue().getAChild*() = e
select tv,
"Delegated $@ for `JsonWebTokenHandler` is $@.",
tv, tv.getTarget().toString(), e, "a callable that always returns the 1st argument"
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
<!DOCTYPE qhelp PUBLIC
"-//Semmle//qhelp//EN"
"qhelp.dtd">
<qhelp>
<overview>
<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>

</overview>
<recommendation>
<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>.
</p>
</recommendation>

<example>
<p>This example delegates <code>AudienceValidator</code> to a callable that always returns true.</p>
<sample src="examples/delegated-security-validations-always-return-true-bad.cs" />

<p>To fix it, use a callable that performs a validation, and fails when appropriate.</p>
<sample src="examples/delegated-security-validations-always-return-true-good.cs" />

</example>

<references>

<li><a href="https://aka.ms/wilson/tokenvalidation">azure-activedirectory-identitymodel-extensions-for-dotnet ValidatingTokens wiki</a></li>

</references>
</qhelp>
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
/**
* @name Delegated security sensitive validations for JsonWebTokenHandler always return true, medium precision
* @description Security sensitive validations for `JsonWebTokenHandler` are being delegated to a function that seems to always return true.
* Higher precision version checks for exception throws, so less false positives are expected.
* @kind problem
* @tags security
* wilson-library
* manual-verification-required
* @id cs/wilson-library/delegated-security-validations-always-return-true
* @problem.severity warning
* @precision high
*/

import csharp
import DataFlow
import wilsonlib

from
TokenValidationParametersPropertyWriteToValidationDelegated tv, Assignment a,
AbstractCallableAlwaysReturnsTrueHigherPrecision e
where a.getLValue() = tv and a.getRValue().getAChild*() = e
select tv,
"JsonWebTokenHandler security-sensitive property $@ is being delegated to $@.",
tv, tv.getTarget().toString(), e, "a callable that always returns \"true\""
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
TokenValidationParameters tokenValidationParamsSignature = new TokenValidationParameters
{
SignatureValidator = (string token, TokenValidationParameters validationParams) => { return new JsonWebToken(token); }
};
Loading