Merge pull request #16559 from hvitved/csharp/callable-always-returns…

…-true C#: Simplify logic in `JsonWebTokenHandlerLib.qll`
github · May 23, 2024 · f517c00 · f517c00
2 parents d202355 + 39019b3
commit f517c00
Show file tree

Hide file tree

Showing 2 changed files with 4 additions and 47 deletions.
diff --git a/csharp/ql/src/experimental/Security Features/JsonWebTokenHandler/JsonWebTokenHandlerLib.qll b/csharp/ql/src/experimental/Security Features/JsonWebTokenHandler/JsonWebTokenHandlerLib.qll
@@ -125,39 +125,24 @@ class TokenValidationParametersProperty extends Property {
 predicate callableHasAReturnStmtAndAlwaysReturnsTrue(Callable c) {
   c.getReturnType() instanceof BoolType and
   not callableMayThrowException(c) and
-  forall(ReturnStmt rs | rs.getEnclosingCallable() = c |
+  forex(ReturnStmt rs | rs.getEnclosingCallable() = c |
     rs.getNumberOfChildren() = 1 and
     isExpressionAlwaysTrue(rs.getChildExpr(0))
-  ) and
-  exists(ReturnStmt rs | rs.getEnclosingCallable() = c)
+  )
 }
 
 /**
  * Holds if the lambda expression `le` always returns true
  */
 predicate lambdaExprReturnsOnlyLiteralTrue(AnonymousFunctionExpr le) {
-  le.getExpressionBody().(BoolLiteral).getBoolValue() = true
-  or
-  // special scenarios where the expression is not a `BoolLiteral`, but it will evaluatue to `true`
-  exists(Expr e | le.getExpressionBody() = e |
-    not e instanceof Call and
-    not e instanceof Literal and
-    e.getType() instanceof BoolType and
-    e.getValue() = "true"
-  )
+  isExpressionAlwaysTrue(le.getExpressionBody())
 }
 
 class CallableAlwaysReturnsTrue extends Callable {
   CallableAlwaysReturnsTrue() {
     callableHasAReturnStmtAndAlwaysReturnsTrue(this)
     or
     lambdaExprReturnsOnlyLiteralTrue(this)
-    or
-    exists(AnonymousFunctionExpr le, Call call, Callable callable | this = le |
-      callable.getACall() = call and
-      call = le.getExpressionBody() and
-      callableHasAReturnStmtAndAlwaysReturnsTrue(callable)
-    )
   }
 }
 
@@ -171,32 +156,6 @@ predicate callableOnlyThrowsArgumentNullException(Callable c) {
   )
 }
 
-/**
- * A specialization of `CallableAlwaysReturnsTrue` that takes into consideration exceptions being thrown for higher precision.
- */
-class CallableAlwaysReturnsTrueHigherPrecision extends CallableAlwaysReturnsTrue {
-  CallableAlwaysReturnsTrueHigherPrecision() {
-    callableOnlyThrowsArgumentNullException(this) and
-    (
-      forall(Call call, Callable callable | call.getEnclosingCallable() = this |
-        callable.getACall() = call and
-        callable instanceof CallableAlwaysReturnsTrueHigherPrecision
-      )
-      or
-      exists(AnonymousFunctionExpr le, Call call, CallableAlwaysReturnsTrueHigherPrecision cat |
-        this = le
-      |
-        le.canReturn(call) and
-        cat.getACall() = call
-      )
-      or
-      exists(LambdaExpr le | le = this |
-        le.getBody() instanceof CallableAlwaysReturnsTrueHigherPrecision
-      )
-    )
-  }
-}
-
 /**
  * A callable that returns a `string` and has a `string` as 1st argument
  */

diff --git a/...ecurity Features/JsonWebTokenHandler/delegated-security-validations-always-return-true.ql b/...ecurity Features/JsonWebTokenHandler/delegated-security-validations-always-return-true.ql
@@ -17,9 +17,7 @@ import DataFlow
 import JsonWebTokenHandlerLib
 import semmle.code.csharp.commons.QualifiedName
 
-from
-  TokenValidationParametersProperty p, CallableAlwaysReturnsTrueHigherPrecision e, string qualifier,
-  string name
+from TokenValidationParametersProperty p, CallableAlwaysReturnsTrue e, string qualifier, string name
 where e = p.getAnAssignedValue() and p.hasFullyQualifiedName(qualifier, name)
 select e,
   "JsonWebTokenHandler security-sensitive property $@ is being delegated to this callable that always returns \"true\".",