mirror of
https://github.com/github/codeql.git
synced 2026-05-03 12:45:27 +02:00
Add a default sanitizer guard for list of constants comparison
Currently it only deals with the case of a switch statement in a function.
This commit is contained in:
@@ -1320,3 +1320,126 @@ private predicate onlyPossibleReturnOfNil(FuncDecl fd, FunctionOutput res, DataF
|
||||
isCertainlyNotNil(otherRet)
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
* Gets a predecessor of `succ` without following edges corresponding to
|
||||
* passing constant case tests in the switch block which is switching on
|
||||
* `protectedExpr`.
|
||||
*/
|
||||
private ControlFlow::Node getANonTestPassingPredecessor(ControlFlow::Node succ, Expr protectedExpr) {
|
||||
result = succ.getAPredecessor() and
|
||||
not exists(Expr testExpr |
|
||||
ControlFlow::isSwitchCaseTestPassingEdge(result, succ, protectedExpr, testExpr) and
|
||||
testExpr.isConst()
|
||||
)
|
||||
}
|
||||
|
||||
private ControlFlow::Node getANonTestPassingReachingNodeRecursive(
|
||||
ControlFlow::Node n, Expr protectedExpr
|
||||
) {
|
||||
result = n or
|
||||
result =
|
||||
getANonTestPassingReachingNodeRecursive(getANonTestPassingPredecessor(n, protectedExpr),
|
||||
protectedExpr)
|
||||
}
|
||||
|
||||
/**
|
||||
* Gets a node by following predecessors from `ret` without following edges
|
||||
* corresponding to passing constant test cases in switch blocks.
|
||||
*/
|
||||
private ControlFlow::Node getANonTestPassingReachingNodeBase(
|
||||
IR::ReturnInstruction ret, Expr protectedExpr
|
||||
) {
|
||||
protectedExpr.getEnclosingFunction() = ret.getReturnStmt().getEnclosingFunction() and
|
||||
result = getANonTestPassingReachingNodeRecursive(ret, protectedExpr)
|
||||
}
|
||||
|
||||
/**
|
||||
* Holds if every way to get from the entry node of the function to `ret`
|
||||
* involves passing a constant test case in the switch statement switching on
|
||||
* `protectedExpr`.
|
||||
*/
|
||||
private predicate mustPassConstantCaseTestToReach(IR::ReturnInstruction ret, Expr protectedExpr) {
|
||||
not exists(ControlFlow::Node entry | entry = ret.getRoot().getEntryNode() |
|
||||
entry = getANonTestPassingReachingNodeBase(ret, protectedExpr)
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
* 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.
|
||||
*
|
||||
* 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.)
|
||||
*/
|
||||
predicate isListOfConstantsComparisonUsingFunctionSwitch(
|
||||
Function f, FunctionInput inp, FunctionOutput outp, DataFlow::Property p
|
||||
) {
|
||||
outp.isResult(_) and
|
||||
exists(FuncDecl fd, ExpressionSwitchStmt ess, Node exprNode |
|
||||
fd.getFunction() = f and
|
||||
exprNode.asExpr() = ess.getExpr() and
|
||||
localFlow(inp.getExitNode(fd), exprNode)
|
||||
|
|
||||
exists(boolean b |
|
||||
p.isBoolean(b) and
|
||||
forex(DataFlow::Node ret, IR::ReturnInstruction ri |
|
||||
ret = outp.getEntryNode(f.getFuncDecl()) and
|
||||
ri.getReturnStmt().getAnExpr() = ret.asExpr() and
|
||||
possiblyReturnsBool(fd, outp, ret, b)
|
||||
|
|
||||
mustPassConstantCaseTestToReach(ri, ess.getExpr())
|
||||
)
|
||||
)
|
||||
or
|
||||
p.isNonNil() and
|
||||
forex(DataFlow::Node ret, IR::ReturnInstruction ri |
|
||||
ret = outp.getEntryNode(f.getFuncDecl()) and
|
||||
ri.getReturnStmt().getAnExpr() = ret.asExpr() and
|
||||
possiblyReturnsNonNil(fd, outp, ret)
|
||||
|
|
||||
mustPassConstantCaseTestToReach(ri, ess.getExpr())
|
||||
)
|
||||
or
|
||||
p.isNil() and
|
||||
forex(DataFlow::Node ret, IR::ReturnInstruction ri |
|
||||
ret = outp.getEntryNode(f.getFuncDecl()) and
|
||||
ri.getReturnStmt().getAnExpr() = ret.asExpr() and
|
||||
ret.asExpr() = Builtin::nil().getAReference()
|
||||
|
|
||||
mustPassConstantCaseTestToReach(ri, ess.getExpr())
|
||||
)
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
* A comparison against a list of constants, acting as a sanitizer guard for
|
||||
* `guardedExpr` by restricting it to a known value.
|
||||
*
|
||||
* Currently this only looks for functions containing a switch statement, but
|
||||
* it could equally look for a check for membership of a constant map or
|
||||
* constant array, which does not need to be in its own function.
|
||||
*/
|
||||
class ListOfConstantsComparisonSanitizerGuard extends TaintTracking::DefaultTaintSanitizerGuard {
|
||||
DataFlow::Node guardedExpr;
|
||||
boolean outcome;
|
||||
|
||||
ListOfConstantsComparisonSanitizerGuard() {
|
||||
exists(
|
||||
Function f, FunctionInput inp, FunctionOutput outp, DataFlow::CallNode call,
|
||||
DataFlow::Property p, DataFlow::Node res
|
||||
|
|
||||
isListOfConstantsComparisonUsingFunctionSwitch(f, inp, outp, p) and
|
||||
call = f.getACall() and
|
||||
guardedExpr = inp.getNode(call) and
|
||||
p.checkOn(this, outcome, res) and
|
||||
DataFlow::localFlow(outp.getNode(call), res)
|
||||
)
|
||||
}
|
||||
|
||||
override predicate checks(Expr e, boolean branch) {
|
||||
e = guardedExpr.asExpr() and branch = outcome
|
||||
}
|
||||
}
|
||||
|
||||
@@ -0,0 +1,170 @@
|
||||
package main
|
||||
|
||||
import (
|
||||
"crypto/rand"
|
||||
"fmt"
|
||||
)
|
||||
|
||||
const constantGlobalVariable string = "constant global variable"
|
||||
|
||||
// Utilities: a source, a sink and an error struct:
|
||||
|
||||
func source() string {
|
||||
return "tainted"
|
||||
}
|
||||
|
||||
func sink(s string) {}
|
||||
|
||||
func getRandomString() string {
|
||||
buff := make([]byte, 10)
|
||||
rand.Read(buff)
|
||||
return fmt.Sprintf("%x", buff)
|
||||
}
|
||||
|
||||
func getConstantString() string {
|
||||
return "constant return value"
|
||||
}
|
||||
|
||||
type errorString struct {
|
||||
s string
|
||||
}
|
||||
|
||||
func (e *errorString) Error() string {
|
||||
return e.s
|
||||
}
|
||||
|
||||
// Candidate functions which compare the input against a list of constants:
|
||||
|
||||
func switchStatementReturningTrueOnlyWhenConstant(s string) bool {
|
||||
switch s {
|
||||
case constantGlobalVariable, "string literal":
|
||||
return true
|
||||
case getRandomString():
|
||||
return false
|
||||
case "another string literal":
|
||||
return false
|
||||
default:
|
||||
return false
|
||||
}
|
||||
}
|
||||
|
||||
func switchStatementReturningFalseOnlyWhenConstant(r string, s string) bool {
|
||||
switch s {
|
||||
case "string literal":
|
||||
return false
|
||||
case constantGlobalVariable:
|
||||
return false
|
||||
case "another string literal":
|
||||
return true
|
||||
}
|
||||
return true
|
||||
}
|
||||
|
||||
func switchStatementReturningNonNilOnlyWhenConstant(s string) (string, error) {
|
||||
switch s {
|
||||
case constantGlobalVariable, "string literal":
|
||||
return "error", &errorString{"a"}
|
||||
case getRandomString():
|
||||
return "no error", nil
|
||||
case "another string literal":
|
||||
return "no error", nil
|
||||
default:
|
||||
return "no error", nil
|
||||
}
|
||||
}
|
||||
|
||||
func switchStatementReturningNilOnlyWhenConstant(s string) *string {
|
||||
t := s
|
||||
switch t {
|
||||
case "string literal":
|
||||
return nil
|
||||
case constantGlobalVariable, "another string literal":
|
||||
str := "matches random string"
|
||||
return &str
|
||||
}
|
||||
str := "no matches"
|
||||
return &str
|
||||
}
|
||||
|
||||
func switchStatementWithoutUsefulInfo(s string) bool {
|
||||
switch s {
|
||||
case constantGlobalVariable, "string literal":
|
||||
return false
|
||||
case getRandomString():
|
||||
return true
|
||||
default:
|
||||
return false
|
||||
}
|
||||
}
|
||||
|
||||
func switchStatementOverRandomString(s string) bool {
|
||||
switch getRandomString() {
|
||||
case "string literal":
|
||||
return true
|
||||
default:
|
||||
return false
|
||||
}
|
||||
}
|
||||
|
||||
// Tests
|
||||
|
||||
func main() {
|
||||
|
||||
// Switch statements in functions
|
||||
|
||||
{
|
||||
s := source()
|
||||
if switchStatementReturningTrueOnlyWhenConstant(s) {
|
||||
sink(s)
|
||||
} else {
|
||||
sink(s) // $dataflow=s
|
||||
}
|
||||
}
|
||||
|
||||
{
|
||||
s := source()
|
||||
if switchStatementReturningFalseOnlyWhenConstant("", s) {
|
||||
sink(s) // $dataflow=s
|
||||
} else {
|
||||
sink(s)
|
||||
}
|
||||
}
|
||||
|
||||
{
|
||||
s := source()
|
||||
_, err := switchStatementReturningNonNilOnlyWhenConstant(s)
|
||||
if err != nil {
|
||||
sink(s)
|
||||
} else {
|
||||
sink(s) // $dataflow=s
|
||||
}
|
||||
}
|
||||
|
||||
{
|
||||
s := source()
|
||||
if switchStatementReturningNilOnlyWhenConstant(s) == nil {
|
||||
sink(s)
|
||||
} else {
|
||||
sink(s) // $dataflow=s
|
||||
}
|
||||
}
|
||||
|
||||
{
|
||||
s := source()
|
||||
if switchStatementWithoutUsefulInfo(s) {
|
||||
sink(s) // $dataflow=s
|
||||
} else {
|
||||
sink(s) // $dataflow=s
|
||||
}
|
||||
}
|
||||
|
||||
{
|
||||
s := source()
|
||||
if switchStatementOverRandomString(s) {
|
||||
sink(s) // $dataflow=s
|
||||
} else {
|
||||
sink(s) // $dataflow=s
|
||||
}
|
||||
}
|
||||
|
||||
}
|
||||
@@ -0,0 +1,29 @@
|
||||
import go
|
||||
import TestUtilities.InlineExpectationsTest
|
||||
|
||||
class TestConfig extends TaintTracking::Configuration {
|
||||
TestConfig() { this = "test config" }
|
||||
|
||||
override predicate isSource(DataFlow::Node source) {
|
||||
source.(DataFlow::CallNode).getTarget().getName() = "source"
|
||||
}
|
||||
|
||||
override predicate isSink(DataFlow::Node sink) {
|
||||
sink = any(DataFlow::CallNode c | c.getTarget().getName() = "sink").getAnArgument()
|
||||
}
|
||||
}
|
||||
|
||||
class DataFlowTest extends InlineExpectationsTest {
|
||||
DataFlowTest() { this = "DataFlowTest" }
|
||||
|
||||
override string getARelevantTag() { result = "dataflow" }
|
||||
|
||||
override predicate hasActualResult(string file, int line, string element, string tag, string value) {
|
||||
tag = "dataflow" and
|
||||
exists(DataFlow::Node sink | any(TestConfig c).hasFlow(_, sink) |
|
||||
element = sink.toString() and
|
||||
value = sink.toString() and
|
||||
sink.hasLocationInfo(file, line, _, _, _)
|
||||
)
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user