From c27175598512499ec5a8008272c08d791f2290d9 Mon Sep 17 00:00:00 2001 From: Owen Mansel-Chan Date: Tue, 10 Mar 2026 22:04:06 +0000 Subject: [PATCH] Add and use `exprRefersToNil` predicate --- go/ql/lib/semmle/go/Expr.qll | 5 ++++- go/ql/lib/semmle/go/dataflow/Properties.qll | 2 +- go/ql/lib/semmle/go/dataflow/internal/DataFlowUtil.qll | 6 +++--- go/ql/lib/semmle/go/dataflow/internal/TaintTrackingUtil.qll | 2 +- go/ql/src/RedundantCode/UnreachableStatement.ql | 2 +- go/ql/src/experimental/CWE-203/Timing.ql | 6 +++--- 6 files changed, 13 insertions(+), 10 deletions(-) diff --git a/go/ql/lib/semmle/go/Expr.qll b/go/ql/lib/semmle/go/Expr.qll index 4a4ab00f0538..53097c7a962d 100644 --- a/go/ql/lib/semmle/go/Expr.qll +++ b/go/ql/lib/semmle/go/Expr.qll @@ -2035,6 +2035,9 @@ class ConstantName extends ValueName { override string getAPrimaryQlClass() { result = "ConstantName" } } +/** Holds if `e` is an expression that refers to the `nil` constant. */ +predicate exprRefersToNil(Expr e) { e.(ConstantName).getTarget() = Builtin::nil() } + /** * A name referring to a variable. * @@ -2175,7 +2178,7 @@ private predicate isTypeExprTopDown(Expr e) { or e = any(TypeSwitchStmt s).getACase().getExpr(_) and // special case: `nil` is allowed in a type case but isn't a type - not e = Builtin::nil().getAReference() + not exprRefersToNil(e) or e = any(SelectorExpr sel | isTypeExprTopDown(sel)).getBase() or diff --git a/go/ql/lib/semmle/go/dataflow/Properties.qll b/go/ql/lib/semmle/go/dataflow/Properties.qll index f7df3391f40b..735deb19c9ff 100644 --- a/go/ql/lib/semmle/go/dataflow/Properties.qll +++ b/go/ql/lib/semmle/go/dataflow/Properties.qll @@ -22,7 +22,7 @@ class Property extends TProperty { isTrue = eq.getPolarity().booleanXor(e.getBoolValue().booleanXor(outcome)) or this = IsNil(isTrue) and - e = Builtin::nil().getAReference() and + exprRefersToNil(e) and isTrue = eq.getPolarity().booleanXor(outcome).booleanNot() ) or diff --git a/go/ql/lib/semmle/go/dataflow/internal/DataFlowUtil.qll b/go/ql/lib/semmle/go/dataflow/internal/DataFlowUtil.qll index 404eca4b4a25..ea1fc575076d 100644 --- a/go/ql/lib/semmle/go/dataflow/internal/DataFlowUtil.qll +++ b/go/ql/lib/semmle/go/dataflow/internal/DataFlowUtil.qll @@ -560,7 +560,7 @@ private predicate onlyPossibleReturnOfBool(FuncDecl fd, FunctionOutput res, Node */ predicate possiblyReturnsNonNil(FuncDecl fd, FunctionOutput res, Node ret) { ret = res.getEntryNode(fd) and - not ret.asExpr() = Builtin::nil().getAReference() + not exprRefersToNil(ret.asExpr()) } /** @@ -570,7 +570,7 @@ predicate possiblyReturnsNonNil(FuncDecl fd, FunctionOutput res, Node ret) { private predicate onlyPossibleReturnOfNonNil(FuncDecl fd, FunctionOutput res, Node ret) { possiblyReturnsNonNil(fd, res, ret) and forall(Node otherRet | otherRet = res.getEntryNode(fd) and otherRet != ret | - otherRet.asExpr() = Builtin::nil().getAReference() + exprRefersToNil(otherRet.asExpr()) ) } @@ -609,7 +609,7 @@ private predicate isCertainlyNotNil(DataFlow::Node node) { */ private predicate onlyPossibleReturnOfNil(FuncDecl fd, FunctionOutput res, DataFlow::Node ret) { ret = res.getEntryNode(fd) and - ret.asExpr() = Builtin::nil().getAReference() and + exprRefersToNil(ret.asExpr()) and forall(DataFlow::Node otherRet | otherRet = res.getEntryNode(fd) and otherRet != ret | isCertainlyNotNil(otherRet) ) diff --git a/go/ql/lib/semmle/go/dataflow/internal/TaintTrackingUtil.qll b/go/ql/lib/semmle/go/dataflow/internal/TaintTrackingUtil.qll index af28f7f40200..adb3d5dcac7b 100644 --- a/go/ql/lib/semmle/go/dataflow/internal/TaintTrackingUtil.qll +++ b/go/ql/lib/semmle/go/dataflow/internal/TaintTrackingUtil.qll @@ -418,7 +418,7 @@ predicate functionEnsuresInputIsConstant( forex(DataFlow::Node ret, IR::ReturnInstruction ri | ret = outp.getEntryNode(fd) and ri.getReturnStmt().getAnExpr() = ret.asExpr() and - ret.asExpr() = Builtin::nil().getAReference() + exprRefersToNil(ret.asExpr()) | DataFlow::localFlow(inp.getExitNode(fd), _) and mustPassConstantCaseTestToReach(ri, inp.getExitNode(fd)) diff --git a/go/ql/src/RedundantCode/UnreachableStatement.ql b/go/ql/src/RedundantCode/UnreachableStatement.ql index c177705a86f4..12b035049e9e 100644 --- a/go/ql/src/RedundantCode/UnreachableStatement.ql +++ b/go/ql/src/RedundantCode/UnreachableStatement.ql @@ -26,7 +26,7 @@ ControlFlow::Node nonGuardPredecessor(ControlFlow::Node nd) { * Matches if `retval` is a constant or a struct composed wholly of constants. */ predicate isAllowedReturnValue(Expr retval) { - retval = Builtin::nil().getAReference() + exprRefersToNil(retval) or retval = Builtin::true_().getAReference() or diff --git a/go/ql/src/experimental/CWE-203/Timing.ql b/go/ql/src/experimental/CWE-203/Timing.ql index e488adf2f97f..30ce6952e067 100644 --- a/go/ql/src/experimental/CWE-203/Timing.ql +++ b/go/ql/src/experimental/CWE-203/Timing.ql @@ -36,7 +36,7 @@ private class SensitiveStringCompareSink extends Sink { not op1 = nonSensitiveOperand and not ( // Comparisons with `nil` should be excluded. - nonSensitiveOperand = Builtin::nil().getAReference() + exprRefersToNil(nonSensitiveOperand) or // Comparisons with empty string should also be excluded. nonSensitiveOperand.getStringValue().length() = 0 @@ -60,7 +60,7 @@ private class SensitiveCompareSink extends Sink { not op1 = op2 and not ( // Comparisons with `nil` should be excluded. - op2 = Builtin::nil().getAReference() + exprRefersToNil(op2) or // Comparisons with empty string should also be excluded. op2.getStringValue().length() = 0 @@ -85,7 +85,7 @@ private class SensitiveStringSink extends Sink { not op1 = op2 and not ( // Comparisons with `nil` should be excluded. - op2 = Builtin::nil().getAReference() + exprRefersToNil(op2) or // Comparisons with empty string should also be excluded. op2.getStringValue().length() = 0