making changes based on feedback during PR

Author: raulgarciamsft

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,27 +18,29 @@ 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() {
     this = "FlowsToTokenValidationResultIsValidCall"
   }
 
   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

csharp/ql/test/experimental/Security Features/JsonWebTokenHandler/delegation-test.cs

@@ -3,9 +3,9 @@
 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)
         {

csharp/ql/test/experimental/Security Features/JsonWebTokenHandler/security-validation-disabled-test.cs

@@ -2,9 +2,9 @@
 using System.Collections.Generic;
 using Microsoft.IdentityModel.Tokens;
 
-namespace WilsonTest
+namespace JsonWebTokenHandlerTest
 {
-    public class Wilson_02
+    public class JsonWebTokenHandler_class01
     {
         public void TestCase01()
         {

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 |