From acd6f4156bc5fbf87f53d3dd3ddc75f695dbd19d Mon Sep 17 00:00:00 2001 From: Tom Hvitved Date: Thu, 5 Mar 2026 09:10:43 +0100 Subject: [PATCH] C#: Avoid computing full TC in `DangerousNonShortCircuitLogic.ql` --- .../DangerousNonShortCircuitLogic.ql | 51 ++++++++++++------- 1 file changed, 32 insertions(+), 19 deletions(-) diff --git a/csharp/ql/src/Likely Bugs/DangerousNonShortCircuitLogic.ql b/csharp/ql/src/Likely Bugs/DangerousNonShortCircuitLogic.ql index d40a28450fc..b980bfba1ae 100644 --- a/csharp/ql/src/Likely Bugs/DangerousNonShortCircuitLogic.ql +++ b/csharp/ql/src/Likely Bugs/DangerousNonShortCircuitLogic.ql @@ -15,23 +15,6 @@ import csharp -/** An expression containing a qualified member access, a method call, or an array access. */ -class DangerousExpression extends Expr { - DangerousExpression() { - exists(Expr e | this = e.getParent*() | - exists(Expr q | q = e.(MemberAccess).getQualifier() | - not q instanceof ThisAccess and - not q instanceof BaseAccess - ) - or - e instanceof MethodCall - or - e instanceof ArrayAccess - ) and - not exists(Expr e | this = e.getParent*() | e.(Call).getTarget().getAParameter().isOutOrRef()) - } -} - /** A use of `&` or `|` on operands of type boolean. */ class NonShortCircuit extends BinaryBitwiseOperation { NonShortCircuit() { @@ -42,10 +25,40 @@ class NonShortCircuit extends BinaryBitwiseOperation { ) and not exists(AssignBitwiseOperation abo | abo.getExpandedAssignment().getRValue() = this) and this.getLeftOperand().getType() instanceof BoolType and - this.getRightOperand().getType() instanceof BoolType and - this.getRightOperand() instanceof DangerousExpression + this.getRightOperand().getType() instanceof BoolType + } + + pragma[nomagic] + private predicate hasRightOperandDescendant(Expr e) { + e = this.getRightOperand() + or + exists(Expr parent | + this.hasRightOperandDescendant(parent) and + e.getParent() = parent + ) + } + + /** + * Holds if this non-short-circuit expression contains a qualified member access, + * a method call, or an array access inside the right operand. + */ + predicate isDangerous() { + exists(Expr e | this.hasRightOperandDescendant(e) | + exists(Expr q | q = e.(MemberAccess).getQualifier() | + not q instanceof ThisAccess and + not q instanceof BaseAccess + ) + or + e instanceof MethodCall + or + e instanceof ArrayAccess + ) and + not exists(Expr e | this.hasRightOperandDescendant(e) | + e.(Call).getTarget().getAParameter().isOutOrRef() + ) } } from NonShortCircuit e +where e.isDangerous() select e, "Potentially dangerous use of non-short circuit logic."