From 760d89b0d3ade1150175188789e69765b96d1737 Mon Sep 17 00:00:00 2001 From: Owen Mansel-Chan <62447351+owen-mc@users.noreply.github.com> Date: Wed, 3 Feb 2021 14:31:34 +0000 Subject: [PATCH] Apply suggestions from code review Co-authored-by: Chris Smowton --- ql/src/semmle/go/controlflow/IR.qll | 1 + .../semmle/go/dataflow/internal/DataFlowUtil.qll | 15 ++++++++------- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/ql/src/semmle/go/controlflow/IR.qll b/ql/src/semmle/go/controlflow/IR.qll index 319d2193d00..95270055d18 100644 --- a/ql/src/semmle/go/controlflow/IR.qll +++ b/ql/src/semmle/go/controlflow/IR.qll @@ -905,6 +905,7 @@ module IR { ReturnInstruction() { this = MkReturnNode(ret) } + /** Gets the corresponding `ReturnStmt`. */ ReturnStmt getReturnStmt() { result = ret } /** Holds if this statement returns multiple results. */ diff --git a/ql/src/semmle/go/dataflow/internal/DataFlowUtil.qll b/ql/src/semmle/go/dataflow/internal/DataFlowUtil.qll index 3bb31ed5c85..bdb4ab046b5 100644 --- a/ql/src/semmle/go/dataflow/internal/DataFlowUtil.qll +++ b/ql/src/semmle/go/dataflow/internal/DataFlowUtil.qll @@ -1367,14 +1367,15 @@ private predicate mustPassConstantCaseTestToReach(IR::ReturnInstruction ret, Exp /** * Holds if whenever `outp` of function `f` satisfies `p`, the input `inp` of - * `f` was compared to a constant in a case clause of a switch statement. + * `f` matched a constant in a case clause of a switch statement. * - * We check this by looking for guards on `inp` that dominate a `return` statement that - * is the only `return` in `f` that can return `true`. This means that if `f` returns `true`, - * the guard must have been satisfied. (Similar reasoning is applied for statements returning - * `false`, `nil` or a non-`nil` value.) + * We check this by looking for guards on `inp` that collectively dominate a + * `return` statement that is the only `return` in `f` that can return `true`. + * This means that if `f` returns `true`, one of the guards must have been + * satisfied. (Similar reasoning is applied for statements returning `false`, + * `nil` or a non-`nil` value.) */ -predicate isListOfConstantsComparisonUsingFunctionSwitch( +predicate functionEnsuresInputIsConstant( Function f, FunctionInput inp, FunctionOutput outp, DataFlow::Property p ) { outp.isResult(_) and @@ -1431,7 +1432,7 @@ class ListOfConstantsComparisonSanitizerGuard extends TaintTracking::DefaultTain Function f, FunctionInput inp, FunctionOutput outp, DataFlow::CallNode call, DataFlow::Property p, DataFlow::Node res | - isListOfConstantsComparisonUsingFunctionSwitch(f, inp, outp, p) and + functionEnsuresInputIsConstant(f, inp, outp, p) and call = f.getACall() and guardedExpr = inp.getNode(call) and p.checkOn(this, outcome, res) and