diff --git a/csharp/ql/src/experimental/Security Features/JsonWebTokenHandler/JsonWebTokenHandlerLib.qll b/csharp/ql/src/experimental/Security Features/JsonWebTokenHandler/JsonWebTokenHandlerLib.qll index b2f3a2fab16..3247a2e4fb6 100644 --- a/csharp/ql/src/experimental/Security Features/JsonWebTokenHandler/JsonWebTokenHandlerLib.qll +++ b/csharp/ql/src/experimental/Security Features/JsonWebTokenHandler/JsonWebTokenHandlerLib.qll @@ -2,7 +2,7 @@ import csharp import DataFlow /** - * Abstract PropertyWrite for `TokenValidationParameters`. + * An abstract PropertyWrite for `TokenValidationParameters`. * Not really necessary anymore, but keeping it in case we want to extend the queries to check on other properties. */ abstract class TokenValidationParametersPropertyWrite extends PropertyWrite { } @@ -18,14 +18,14 @@ class TokenValidationParametersPropertyWriteToBypassSensitiveValidation extends p.getAnAccess() = this and c.getAProperty() = p and p.getName() in [ - "ValidateIssuer", "ValidateAudience", "ValidateLifetime", "RequireExpirationTime" + "ValidateIssuer", "ValidateAudience", "ValidateLifetime", "RequireExpirationTime", "RequireAudience" ] ) } } /** - * Dataflow from a `false` value to an to a write sensitive property for `TokenValidationParameters`. + * A dataflow from a `false` value to a write sensitive property for `TokenValidationParameters`. */ class FalseValueFlowsToTokenValidationParametersPropertyWriteToBypassValidation extends TaintTracking::Configuration { FalseValueFlowsToTokenValidationParametersPropertyWriteToBypassValidation() { @@ -33,12 +33,14 @@ class FalseValueFlowsToTokenValidationParametersPropertyWriteToBypassValidation } override predicate isSource(DataFlow::Node source) { - source.asExpr().(BoolLiteral).getValue() = "false" + source.asExpr().getValue() = "false" and + source.asExpr().getType() instanceof BoolType } override predicate isSink(DataFlow::Node sink) { - exists(TokenValidationParametersPropertyWrite pw, Assignment a | a.getLValue() = pw | + exists(Assignment a | sink.asExpr() = a.getRValue() + and a.getLValue() instanceof TokenValidationParametersPropertyWrite ) } } @@ -55,7 +57,7 @@ predicate isAssemblyOlderVersion(string assemblyName, string ver) { } /** - * Method `ValidateToken` for `Microsoft.IdentityModel.JsonWebTokens.JsonWebTokenHandler` or other Token handler that shares the same behavior characteristics + * A method `ValidateToken` for `Microsoft.IdentityModel.JsonWebTokens.JsonWebTokenHandler` or other Token handler that shares the same behavior characteristics */ class JsonWebTokenHandlerValidateTokenMethod extends Method { JsonWebTokenHandlerValidateTokenMethod() { @@ -78,7 +80,7 @@ class JsonWebTokenHandlerValidateTokenCall extends MethodCall { } /** - * Read access for properties `IsValid` or `Exception` for `Microsoft.IdentityModel.JsonWebTokens.JsonWebTokenHandler.ValidateToken` + * A read access for properties `IsValid` or `Exception` for `Microsoft.IdentityModel.JsonWebTokens.JsonWebTokenHandler.ValidateToken` */ class TokenValidationResultIsValidCall extends PropertyRead { TokenValidationResultIsValidCall() { @@ -116,7 +118,7 @@ predicate hasAFlowToTokenValidationResultIsValidCall(JsonWebTokenHandlerValidate } /** - * Property write for security-sensitive properties for `Microsoft.IdentityModel.Tokens.TokenValidationParameters` + * A property write for security-sensitive properties for `Microsoft.IdentityModel.Tokens.TokenValidationParameters` */ class TokenValidationParametersPropertyWriteToValidationDelegated extends PropertyWrite { TokenValidationParametersPropertyWriteToValidationDelegated() { @@ -136,7 +138,7 @@ class TokenValidationParametersPropertyWriteToValidationDelegated extends Proper /** * Holds if the callable has a return statement and it always returns true for all such statements */ -predicate callableHasARetrunStmtAndAlwaysReturnsTrue(Callable c) { +predicate callableHasAReturnStmtAndAlwaysReturnsTrue(Callable c) { c.getReturnType().toString() = "Boolean" and forall(ReturnStmt rs | rs.getEnclosingCallable() = c | rs.getChildExpr(0).(BoolLiteral).getBoolValue() = true @@ -153,7 +155,7 @@ predicate lambdaExprReturnsOnlyLiteralTrue(LambdaExpr le) { class CallableAlwaysReturnsTrue extends Callable { CallableAlwaysReturnsTrue() { - callableHasARetrunStmtAndAlwaysReturnsTrue(this) + callableHasAReturnStmtAndAlwaysReturnsTrue(this) or lambdaExprReturnsOnlyLiteralTrue(this) or @@ -195,7 +197,7 @@ class CallableAlwaysReturnsTrueHigherPrecision extends CallableAlwaysReturnsTrue } /** - * Property Write for the `IssuerValidator` property for `Microsoft.IdentityModel.Tokens.TokenValidationParameters` + * A property Write for the `IssuerValidator` property for `Microsoft.IdentityModel.Tokens.TokenValidationParameters` */ class TokenValidationParametersPropertyWriteToValidationDelegatedIssuerValidator extends PropertyWrite { TokenValidationParametersPropertyWriteToValidationDelegatedIssuerValidator() { @@ -204,7 +206,7 @@ class TokenValidationParametersPropertyWriteToValidationDelegatedIssuerValidator | p.getAnAccess() = this and c.getAProperty() = p and - p.getName() in ["IssuerValidator"] + p.hasName("IssuerValidator") ) } } @@ -214,22 +216,22 @@ class TokenValidationParametersPropertyWriteToValidationDelegatedIssuerValidator */ private class CallableReturnsStringAndArg0IsString extends Callable { CallableReturnsStringAndArg0IsString() { - this.getReturnType().toString() = "String" and + this.getReturnType() instanceof StringType and this.getParameter(0).getType().toString() = "String" } } /** - * A Callable that always retrun the 1st argument, both of `string` type + * A Callable that always return the 1st argument, both of `string` type */ -class CallableAlwatsReturnsParameter0 extends CallableReturnsStringAndArg0IsString { - CallableAlwatsReturnsParameter0() { +class CallableAlwaysReturnsParameter0 extends CallableReturnsStringAndArg0IsString { + CallableAlwaysReturnsParameter0() { forall(ReturnStmt rs | rs.getEnclosingCallable() = this | rs.getChild(0) = this.getParameter(0).getAnAccess() ) and exists(ReturnStmt rs | rs.getEnclosingCallable() = this) or - exists(LambdaExpr le, Call call, CallableAlwatsReturnsParameter0 cat | this = le | + exists(LambdaExpr le, Call call, CallableAlwaysReturnsParameter0 cat | this = le | call = le.getExpressionBody() and cat.getACall() = call ) @@ -239,17 +241,17 @@ class CallableAlwatsReturnsParameter0 extends CallableReturnsStringAndArg0IsStri } /** - * A Callable that always retrun the 1st argument, both of `string` type. Higher precision + * A Callable that always return the 1st argument, both of `string` type. Higher precision */ -class CallableAlwatsReturnsParameter0MayThrowExceptions extends CallableReturnsStringAndArg0IsString { - CallableAlwatsReturnsParameter0MayThrowExceptions() { +class CallableAlwaysReturnsParameter0MayThrowExceptions extends CallableReturnsStringAndArg0IsString { + CallableAlwaysReturnsParameter0MayThrowExceptions() { callableOnlyThrowsArgumentNullException(this) and forall(ReturnStmt rs | rs.getEnclosingCallable() = this | rs.getChild(0) = this.getParameter(0).getAnAccess() ) and exists(ReturnStmt rs | rs.getEnclosingCallable() = this) or - exists(LambdaExpr le, Call call, CallableAlwatsReturnsParameter0MayThrowExceptions cat | + exists(LambdaExpr le, Call call, CallableAlwaysReturnsParameter0MayThrowExceptions cat | this = le | call = le.getExpressionBody() and diff --git a/csharp/ql/test/experimental/Security Features/JsonWebTokenHandler/delegation-test.cs b/csharp/ql/test/experimental/Security Features/JsonWebTokenHandler/delegation-test.cs index fe4a5f1d6ea..452cb1dc648 100644 --- a/csharp/ql/test/experimental/Security Features/JsonWebTokenHandler/delegation-test.cs +++ b/csharp/ql/test/experimental/Security Features/JsonWebTokenHandler/delegation-test.cs @@ -3,9 +3,9 @@ using System.Collections.Generic; using Microsoft.IdentityModel.Tokens; using Microsoft.IdentityModel.JsonWebTokens; -namespace WilsonTest +namespace JsonWebTokenHandlerTest { - public class Wilson_03 + public class JsonWebTokenHandler_00 { public static object ThrowIfNull(string name, object value) { diff --git a/csharp/ql/test/experimental/Security Features/JsonWebTokenHandler/security-validation-disabled-test.cs b/csharp/ql/test/experimental/Security Features/JsonWebTokenHandler/security-validation-disabled-test.cs index 149448ce122..e3caddfd227 100644 --- a/csharp/ql/test/experimental/Security Features/JsonWebTokenHandler/security-validation-disabled-test.cs +++ b/csharp/ql/test/experimental/Security Features/JsonWebTokenHandler/security-validation-disabled-test.cs @@ -2,9 +2,9 @@ using System; using System.Collections.Generic; using Microsoft.IdentityModel.Tokens; -namespace WilsonTest +namespace JsonWebTokenHandlerTest { - public class Wilson_02 + public class JsonWebTokenHandler_class01 { public void TestCase01() { diff --git a/csharp/ql/test/experimental/Security Features/JsonWebTokenHandler/security-validation-disabled.expected b/csharp/ql/test/experimental/Security Features/JsonWebTokenHandler/security-validation-disabled.expected index 213c58643ec..071cc2ea2f8 100644 --- a/csharp/ql/test/experimental/Security Features/JsonWebTokenHandler/security-validation-disabled.expected +++ b/csharp/ql/test/experimental/Security Features/JsonWebTokenHandler/security-validation-disabled.expected @@ -2,3 +2,4 @@ | security-validation-disabled-test.cs:32:36:32:40 | false | The security sensitive property $@ is being disabled by the followign value: $@. | security-validation-disabled-test.cs:32:17:32:32 | access to property ValidateAudience | ValidateAudience | security-validation-disabled-test.cs:32:36:32:40 | false | false | | security-validation-disabled-test.cs:33:36:33:40 | false | The security sensitive property $@ is being disabled by the followign value: $@. | security-validation-disabled-test.cs:33:17:33:32 | access to property ValidateLifetime | ValidateLifetime | security-validation-disabled-test.cs:33:36:33:40 | false | false | | security-validation-disabled-test.cs:34:41:34:45 | false | The security sensitive property $@ is being disabled by the followign value: $@. | security-validation-disabled-test.cs:34:17:34:37 | access to property RequireExpirationTime | RequireExpirationTime | security-validation-disabled-test.cs:34:41:34:45 | false | false | +| security-validation-disabled-test.cs:37:35:37:39 | false | The security sensitive property $@ is being disabled by the followign value: $@. | security-validation-disabled-test.cs:37:17:37:31 | access to property RequireAudience | RequireAudience | security-validation-disabled-test.cs:37:35:37:39 | false | false |