Merge pull request #21296 from yoff/python/bool-comparison-guards

Python: Handle guards being compared to boolean literals

Author: yoff

python/ql/lib/change-notes/2026-02-08-guards-compared-to-boolean-literals.md

@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* When a guard such as `isSafe(x)` is defined, we now also automatically handle `isSafe(x) == true` and `isSafe(x) != false`.

python/ql/lib/semmle/python/dataflow/new/internal/DataFlowPublic.qll

@@ -595,14 +595,42 @@ ControlFlowNode guardNode(ConditionBlock conditionBlock, boolean flipped) {
   result = conditionBlock.getLastNode() and
   flipped = false
   or
-  // Recursive case: if a guard node is a `not`-expression,
+  // Recursive cases:
+  // if a guard node is a `not`-expression,
   // the operand is also a guard node, but with inverted polarity.
   exists(UnaryExprNode notNode |
     result = notNode.getOperand() and
     notNode.getNode().getOp() instanceof Not
   |
     notNode = guardNode(conditionBlock, flipped.booleanNot())
   )
+  or
+  // if a guard node is compared to a boolean literal,
+  // the other operand is also a guard node,
+  // but with polarity depending on the literal (and on the comparison).
+  exists(CompareNode cmpNode, Cmpop op, ControlFlowNode b, boolean should_flip |
+    (
+      cmpNode.operands(result, op, b) or
+      cmpNode.operands(b, op, result)
+    ) and
+    not result.getNode() instanceof BooleanLiteral and
+    (
+      // comparing to the boolean
+      (op instanceof Eq or op instanceof Is) and
+      // we should flip if the value compared against, here the value of `b`, is false
+      should_flip = b.getNode().(BooleanLiteral).booleanValue().booleanNot()
+      or
+      // comparing to the negation of the boolean
+      (op instanceof NotEq or op instanceof IsNot) and
+      // again, we should flip if the value compared against, here the value of `not b`, is false.
+      // That is, if the value of `b` is true.
+      should_flip = b.getNode().(BooleanLiteral).booleanValue()
+    )
+  |
+    // we flip `flipped` according to `should_flip` via the formula `flipped xor should_flip`.
+    flipped in [true, false] and
+    cmpNode = guardNode(conditionBlock, flipped.booleanXor(should_flip))
+  )
 }
 
 /**

python/ql/lib/semmle/python/frameworks/AntiSSRF.model.yml

@@ -0,0 +1,6 @@
+extensions:
+  - addsTo:
+      pack: codeql/python-all
+      extensible: barrierGuardModel
+    data:
+      - ['AntiSSRF', 'Member[URIValidator].Member[in_domain,in_azure_keyvault_domain,in_azure_storage_domain].Argument[0]', "true", 'request-forgery']

python/ql/lib/semmle/python/security/dataflow/ServerSideRequestForgeryCustomizations.qll

@@ -10,6 +10,7 @@ private import semmle.python.Concepts
 private import semmle.python.dataflow.new.RemoteFlowSources
 private import semmle.python.dataflow.new.BarrierGuards
 private import semmle.python.ApiGraphs
+private import semmle.python.frameworks.data.internal.ApiGraphModels
 
 /**
  * Provides default sources, sinks and sanitizers for detecting
@@ -177,35 +178,7 @@ module ServerSideRequestForgery {
     )
   }
 
-  /** A validation of a URI using the `AntiSSRF` library, considered as a full-ssrf sanitizer. */
-  private class UriValidator extends FullUrlControlSanitizer {
-    UriValidator() { this = DataFlow::BarrierGuard<uri_validator/3>::getABarrierNode() }
-  }
-
-  import semmle.python.dataflow.new.internal.DataFlowPublic
-
-  private predicate uri_validator(DataFlow::GuardNode g, ControlFlowNode node, boolean branch) {
-    exists(DataFlow::CallCfgNode call, string funcs |
-      funcs in ["in_domain", "in_azure_keyvault_domain", "in_azure_storage_domain"] and
-      call = API::moduleImport("AntiSSRF").getMember("URIValidator").getMember(funcs).getACall() and
-      call.getArg(0).asCfgNode() = node
-    |
-      // validator call directly (e.g., if URIValidator.in_domain(...) )
-      g = call.asCfgNode() and
-      branch = true
-      or
-      // validator used in a comparison
-      exists(Cmpop op, Node n, ControlFlowNode l |
-        n.getALocalSource() = call and g.(CompareNode).operands(n.asCfgNode(), op, l)
-      |
-        // validator == true or validator == false or validator is True or validator is False
-        (op instanceof Eq or op instanceof Is) and
-        branch = l.getNode().(BooleanLiteral).booleanValue()
-        or
-        // validator != false or validator != true or validator is not True or validator is not False
-        (op instanceof NotEq or op instanceof IsNot) and
-        branch = l.getNode().(BooleanLiteral).booleanValue().booleanNot()
-      )
-    )
+  private class ExternalRequestForgerySanitizer extends FullUrlControlSanitizer {
+    ExternalRequestForgerySanitizer() { ModelOutput::barrierNode(this, "request-forgery") }
   }
 }

python/ql/test/library-tests/dataflow/tainttracking/customSanitizer/InlineTaintTest.expected

@@ -22,4 +22,12 @@ isSanitizer
 | test_logical.py:176:24:176:24 | ControlFlowNode for s |
 | test_logical.py:185:24:185:24 | ControlFlowNode for s |
 | test_logical.py:193:24:193:24 | ControlFlowNode for s |
+| test_logical.py:199:28:199:28 | ControlFlowNode for s |
+| test_logical.py:206:28:206:28 | ControlFlowNode for s |
+| test_logical.py:211:28:211:28 | ControlFlowNode for s |
+| test_logical.py:214:28:214:28 | ControlFlowNode for s |
+| test_logical.py:219:28:219:28 | ControlFlowNode for s |
+| test_logical.py:226:28:226:28 | ControlFlowNode for s |
+| test_logical.py:231:28:231:28 | ControlFlowNode for s |
+| test_logical.py:234:28:234:28 | ControlFlowNode for s |
 | test_reference.py:31:28:31:28 | ControlFlowNode for s |

python/ql/test/library-tests/dataflow/tainttracking/customSanitizer/test_logical.py

@@ -192,6 +192,49 @@ def test_with_exception_neg():
 
     ensure_not_tainted(s)
 
+def test_comparison_with_bool():
+    s = TAINTED_STRING
+
+    if is_safe(s) == True:
+        ensure_not_tainted(s)
+    else:
+        ensure_tainted(s) # $ tainted
+
+    if is_safe(s) == False:
+        ensure_tainted(s) # $ tainted
+    else:
+        ensure_not_tainted(s)
+
+    if is_safe(s) != True:
+        ensure_tainted(s) # $ tainted
+    else:
+        ensure_not_tainted(s)
+
+    if is_safe(s) != False:
+        ensure_not_tainted(s)
+    else:
+        ensure_tainted(s) # $ tainted
+
+    if is_safe(s) is True:
+        ensure_not_tainted(s)
+    else:
+        ensure_tainted(s) # $ tainted
+
+    if is_safe(s) is False:
+        ensure_tainted(s) # $ tainted
+    else:
+        ensure_not_tainted(s)
+
+    if is_safe(s) is not True:
+        ensure_tainted(s) # $ tainted
+    else:
+        ensure_not_tainted(s)
+
+    if is_safe(s) is not False:
+        ensure_not_tainted(s)
+    else:
+        ensure_tainted(s) # $ tainted   
+
 # Make tests runable
 
 test_basic()
@@ -211,3 +254,4 @@ def test_with_exception_neg():
     test_with_exception_neg()
 except:
     pass
+test_comparison_with_bool()