From 8488039fb9dc8de56a60b56e56b672a540f67963 Mon Sep 17 00:00:00 2001 From: yoff Date: Sun, 8 Feb 2026 09:32:23 +0100 Subject: [PATCH 1/7] python: add tests for guards compared to booleans --- .../customSanitizer/test_logical.py | 44 +++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/python/ql/test/library-tests/dataflow/tainttracking/customSanitizer/test_logical.py b/python/ql/test/library-tests/dataflow/tainttracking/customSanitizer/test_logical.py index 26e69b8fc050..99b5eafad413 100644 --- a/python/ql/test/library-tests/dataflow/tainttracking/customSanitizer/test_logical.py +++ b/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) # $ SPURIOUS: tainted + else: + ensure_tainted(s) # $ tainted + + if is_safe(s) == False: + ensure_tainted(s) # $ tainted + else: + ensure_not_tainted(s) # $ SPURIOUS: tainted + + if is_safe(s) != True: + ensure_tainted(s) # $ tainted + else: + ensure_not_tainted(s) # $ SPURIOUS: tainted + + if is_safe(s) != False: + ensure_not_tainted(s) # $ SPURIOUS: tainted + else: + ensure_tainted(s) # $ tainted + + if is_safe(s) is True: + ensure_not_tainted(s) # $ SPURIOUS: tainted + else: + ensure_tainted(s) # $ tainted + + if is_safe(s) is False: + ensure_tainted(s) # $ tainted + else: + ensure_not_tainted(s) # $ SPURIOUS: tainted + + if is_safe(s) is not True: + ensure_tainted(s) # $ tainted + else: + ensure_not_tainted(s) # $ SPURIOUS: tainted + + if is_safe(s) is not False: + ensure_not_tainted(s) # $ SPURIOUS: tainted + 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() From 7351e82c9221d33c29e2eb609f422ff6d5f2e75b Mon Sep 17 00:00:00 2001 From: yoff Date: Sun, 8 Feb 2026 09:35:13 +0100 Subject: [PATCH 2/7] python: handle guards compared to boolean literals --- .../dataflow/new/internal/DataFlowPublic.qll | 30 ++++++++++++++++++- .../customSanitizer/InlineTaintTest.expected | 8 +++++ .../customSanitizer/test_logical.py | 16 +++++----- 3 files changed, 45 insertions(+), 9 deletions(-) diff --git a/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowPublic.qll b/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowPublic.qll index f63d24a300ca..3b3f7f7daebb 100644 --- a/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowPublic.qll +++ b/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowPublic.qll @@ -595,7 +595,8 @@ 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 @@ -603,6 +604,33 @@ ControlFlowNode guardNode(ConditionBlock conditionBlock, boolean flipped) { | 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 bool | + ( + 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 + // `bool` is the value being compared against, here the value of `b` + b.getNode().(BooleanLiteral).booleanValue() = bool + or + // comparing to the negation of the boolean + (op instanceof NotEq or op instanceof IsNot) and + // again, `bool` is the value being compared against, but here it is the value of `not b` + b.getNode().(BooleanLiteral).booleanValue() = bool.booleanNot() + ) + | + // if `bool` is true, we should preserve `flipped`, otherwise we should flip it + // `flipped xor (not bool)` achieves that. + flipped in [true, false] and + cmpNode = guardNode(conditionBlock, flipped.booleanXor(bool.booleanNot())) + ) } /** diff --git a/python/ql/test/library-tests/dataflow/tainttracking/customSanitizer/InlineTaintTest.expected b/python/ql/test/library-tests/dataflow/tainttracking/customSanitizer/InlineTaintTest.expected index 86d49b2b249b..89849279d446 100644 --- a/python/ql/test/library-tests/dataflow/tainttracking/customSanitizer/InlineTaintTest.expected +++ b/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 | diff --git a/python/ql/test/library-tests/dataflow/tainttracking/customSanitizer/test_logical.py b/python/ql/test/library-tests/dataflow/tainttracking/customSanitizer/test_logical.py index 99b5eafad413..ff215f97f41b 100644 --- a/python/ql/test/library-tests/dataflow/tainttracking/customSanitizer/test_logical.py +++ b/python/ql/test/library-tests/dataflow/tainttracking/customSanitizer/test_logical.py @@ -196,42 +196,42 @@ def test_comparison_with_bool(): s = TAINTED_STRING if is_safe(s) == True: - ensure_not_tainted(s) # $ SPURIOUS: tainted + ensure_not_tainted(s) else: ensure_tainted(s) # $ tainted if is_safe(s) == False: ensure_tainted(s) # $ tainted else: - ensure_not_tainted(s) # $ SPURIOUS: tainted + ensure_not_tainted(s) if is_safe(s) != True: ensure_tainted(s) # $ tainted else: - ensure_not_tainted(s) # $ SPURIOUS: tainted + ensure_not_tainted(s) if is_safe(s) != False: - ensure_not_tainted(s) # $ SPURIOUS: tainted + ensure_not_tainted(s) else: ensure_tainted(s) # $ tainted if is_safe(s) is True: - ensure_not_tainted(s) # $ SPURIOUS: tainted + ensure_not_tainted(s) else: ensure_tainted(s) # $ tainted if is_safe(s) is False: ensure_tainted(s) # $ tainted else: - ensure_not_tainted(s) # $ SPURIOUS: tainted + ensure_not_tainted(s) if is_safe(s) is not True: ensure_tainted(s) # $ tainted else: - ensure_not_tainted(s) # $ SPURIOUS: tainted + ensure_not_tainted(s) if is_safe(s) is not False: - ensure_not_tainted(s) # $ SPURIOUS: tainted + ensure_not_tainted(s) else: ensure_tainted(s) # $ tainted From 7df44f9418489c0ac38af957466590b48beac32c Mon Sep 17 00:00:00 2001 From: yoff Date: Sun, 8 Feb 2026 09:45:32 +0100 Subject: [PATCH 3/7] python: add change note --- .../2026-02-08-guards-compared-to-boolean-literals.md | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 python/ql/lib/change-notes/2026-02-08-guards-compared-to-boolean-literals.md diff --git a/python/ql/lib/change-notes/2026-02-08-guards-compared-to-boolean-literals.md b/python/ql/lib/change-notes/2026-02-08-guards-compared-to-boolean-literals.md new file mode 100644 index 000000000000..bf626c2958c5 --- /dev/null +++ b/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`. From c4f8748a422489724260928a90e129b67acff1c2 Mon Sep 17 00:00:00 2001 From: yoff Date: Wed, 25 Feb 2026 18:03:40 +0100 Subject: [PATCH 4/7] Python: simplify barrier guard --- ...ServerSideRequestForgeryCustomizations.qll | 22 ++++--------------- 1 file changed, 4 insertions(+), 18 deletions(-) diff --git a/python/ql/lib/semmle/python/security/dataflow/ServerSideRequestForgeryCustomizations.qll b/python/ql/lib/semmle/python/security/dataflow/ServerSideRequestForgeryCustomizations.qll index 3fb260e425d3..999778a6f233 100644 --- a/python/ql/lib/semmle/python/security/dataflow/ServerSideRequestForgeryCustomizations.qll +++ b/python/ql/lib/semmle/python/security/dataflow/ServerSideRequestForgeryCustomizations.qll @@ -177,7 +177,6 @@ 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::getABarrierNode() } } @@ -185,27 +184,14 @@ module ServerSideRequestForgery { 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 + exists(DataFlow::CallCfgNode call, string validator_name | + validator_name in ["in_domain", "in_azure_keyvault_domain", "in_azure_storage_domain"] and + call = + API::moduleImport("AntiSSRF").getMember("URIValidator").getMember(validator_name).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() - ) ) } } From 9b9c9304c7e53e1b5aa38b09a2c68baf03c1fdf1 Mon Sep 17 00:00:00 2001 From: yoff Date: Wed, 25 Feb 2026 18:16:38 +0100 Subject: [PATCH 5/7] Python: simplify logic, suggested in review --- .../dataflow/new/internal/DataFlowPublic.qll | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowPublic.qll b/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowPublic.qll index 3b3f7f7daebb..89c5c2d8116a 100644 --- a/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowPublic.qll +++ b/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowPublic.qll @@ -608,7 +608,7 @@ ControlFlowNode guardNode(ConditionBlock conditionBlock, boolean flipped) { // 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 bool | + exists(CompareNode cmpNode, Cmpop op, ControlFlowNode b, boolean should_flip | ( cmpNode.operands(result, op, b) or cmpNode.operands(b, op, result) @@ -617,19 +617,19 @@ ControlFlowNode guardNode(ConditionBlock conditionBlock, boolean flipped) { ( // comparing to the boolean (op instanceof Eq or op instanceof Is) and - // `bool` is the value being compared against, here the value of `b` - b.getNode().(BooleanLiteral).booleanValue() = bool + // we shoould 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, `bool` is the value being compared against, but here it is the value of `not b` - b.getNode().(BooleanLiteral).booleanValue() = bool.booleanNot() + // 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() ) | - // if `bool` is true, we should preserve `flipped`, otherwise we should flip it - // `flipped xor (not bool)` achieves that. + // we flip `flipped` according to `should_flip` via the formula `flipped xor should_flip`. flipped in [true, false] and - cmpNode = guardNode(conditionBlock, flipped.booleanXor(bool.booleanNot())) + cmpNode = guardNode(conditionBlock, flipped.booleanXor(should_flip)) ) } From cfbae5084561cd80f65233747689106f1e579be2 Mon Sep 17 00:00:00 2001 From: yoff Date: Thu, 26 Feb 2026 13:11:43 +0100 Subject: [PATCH 6/7] Python: convert barrier guard to MaD --- .../python/frameworks/AntiSSRF.model.yml | 6 ++++++ ...ServerSideRequestForgeryCustomizations.qll | 19 +++---------------- 2 files changed, 9 insertions(+), 16 deletions(-) create mode 100644 python/ql/lib/semmle/python/frameworks/AntiSSRF.model.yml diff --git a/python/ql/lib/semmle/python/frameworks/AntiSSRF.model.yml b/python/ql/lib/semmle/python/frameworks/AntiSSRF.model.yml new file mode 100644 index 000000000000..42f483c6970e --- /dev/null +++ b/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'] diff --git a/python/ql/lib/semmle/python/security/dataflow/ServerSideRequestForgeryCustomizations.qll b/python/ql/lib/semmle/python/security/dataflow/ServerSideRequestForgeryCustomizations.qll index 999778a6f233..e3f18170f630 100644 --- a/python/ql/lib/semmle/python/security/dataflow/ServerSideRequestForgeryCustomizations.qll +++ b/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,21 +178,7 @@ module ServerSideRequestForgery { ) } - private class UriValidator extends FullUrlControlSanitizer { - UriValidator() { this = DataFlow::BarrierGuard::getABarrierNode() } - } - - import semmle.python.dataflow.new.internal.DataFlowPublic - - private predicate uri_validator(DataFlow::GuardNode g, ControlFlowNode node, boolean branch) { - exists(DataFlow::CallCfgNode call, string validator_name | - validator_name in ["in_domain", "in_azure_keyvault_domain", "in_azure_storage_domain"] and - call = - API::moduleImport("AntiSSRF").getMember("URIValidator").getMember(validator_name).getACall() and - call.getArg(0).asCfgNode() = node - | - g = call.asCfgNode() and - branch = true - ) + private class ExternalRequestForgerySanitizer extends FullUrlControlSanitizer { + ExternalRequestForgerySanitizer() { ModelOutput::barrierNode(this, "request-forgery") } } } From 89e5a9bd728e4f48ad0a3adc31dd0ad374da6a2f Mon Sep 17 00:00:00 2001 From: yoff Date: Thu, 26 Feb 2026 13:14:26 +0100 Subject: [PATCH 7/7] Update python/ql/lib/semmle/python/dataflow/new/internal/DataFlowPublic.qll Co-authored-by: Taus --- .../lib/semmle/python/dataflow/new/internal/DataFlowPublic.qll | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowPublic.qll b/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowPublic.qll index 89c5c2d8116a..8612d4a253e0 100644 --- a/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowPublic.qll +++ b/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowPublic.qll @@ -617,7 +617,7 @@ ControlFlowNode guardNode(ConditionBlock conditionBlock, boolean flipped) { ( // comparing to the boolean (op instanceof Eq or op instanceof Is) and - // we shoould flip if the value compared against, here the value of `b`, is false + // 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