Treat functions that directly return a BarrierGuard like BarrierGuards themselves

This commit is contained in:
Chris Smowton
2020-11-18 18:58:01 +00:00
parent 387a13f22a
commit 2377337564
5 changed files with 968 additions and 23 deletions

View File

@@ -14,12 +14,9 @@ private newtype TProperty =
* Supported properties currently are Boolean truth and `nil`-ness.
*/
class Property extends TProperty {
/**
* Holds if `test` evaluating to `outcome` means that this property holds of `nd`.
*/
predicate checkOn(DataFlow::Node test, Boolean outcome, DataFlow::Node nd) {
private predicate checkOnExpr(Expr test, Boolean outcome, DataFlow::Node nd) {
exists(EqualityTestExpr eq, Expr e, boolean isTrue |
eq = test.asExpr() and eq.hasOperands(nd.asExpr(), e)
eq = test and eq.hasOperands(nd.asExpr(), e)
|
this = IsBoolean(isTrue) and
isTrue = eq.getPolarity().booleanXor(e.getBoolValue().booleanXor(outcome))
@@ -29,15 +26,44 @@ class Property extends TProperty {
isTrue = eq.getPolarity().booleanXor(outcome).booleanNot()
)
or
test = nd and
test.asExpr() instanceof ValueExpr and
// if test = outcome ==> nd matches this
// then !test = !outcome ==> nd matches this
this.checkOnExpr(test.(NotExpr).getOperand(), outcome.booleanNot(), nd)
or
// if test = outcome ==> nd matches this
// then (test) = outcome ==> nd matches this
this.checkOnExpr(test.(ParenExpr).getExpr(), outcome, nd)
or
// if test = true ==> nd matches this
// then (test && e) = true ==> nd matches this
outcome = true and
this.checkOnExpr(test.(LandExpr).getAnOperand(), outcome, nd)
or
// if test = false ==> nd matches this
// then (test || e) = false ==> nd matches this
outcome = false and
this.checkOnExpr(test.(LorExpr).getAnOperand(), outcome, nd)
or
test = nd.asExpr() and
test instanceof ValueExpr and
test.getType().getUnderlyingType() instanceof BoolType and
this = IsBoolean(outcome)
}
/**
* Holds if `test` evaluating to `outcome` means that this property holds of `nd`, where `nd` is a
* subexpression of `test`.
*/
predicate checkOn(DataFlow::Node test, Boolean outcome, DataFlow::Node nd) {
checkOnExpr(test.asExpr(), outcome, nd)
}
/** Holds if this is the property of having the Boolean value `b`. */
predicate isBoolean(boolean b) { this = IsBoolean(b) }
/** Returns the boolean represented by this property if it is a boolean. */
boolean asBoolean() { this = IsBoolean(result) }
/** Holds if this is the property of being `nil`. */
predicate isNil() { this = IsNil(true) }
@@ -58,3 +84,18 @@ class Property extends TProperty {
result = "is not nil"
}
}
/**
* Gets a `Property` representing truth outcome `b`.
*/
Property booleanProperty(boolean b) { result = IsBoolean(b) }
/**
* Gets a `Property` representing `nil`-ness.
*/
Property nilProperty() { result = IsNil(true) }
/**
* Gets a `Property` representing non-`nil`-ness.
*/
Property notNilProperty() { result = IsNil(false) }

View File

@@ -1143,28 +1143,65 @@ abstract class BarrierGuard extends Node {
* 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` or a non-`nil` value.)
* `false`, `nil` or a non-`nil` value.)
*/
private predicate guardingFunction(
Function f, FunctionInput inp, FunctionOutput outp, DataFlow::Property p
) {
exists(ControlFlow::ConditionGuardNode guard, Node arg, FuncDecl fd, Node ret |
exists(FuncDecl fd, Node arg, Node ret |
fd.getFunction() = f and
guards(guard, arg) and
localFlow(inp.getExitNode(fd), arg) and
ret = outp.getEntryNode(fd) and
guard.dominates(ret.getBasicBlock())
|
exists(boolean b |
onlyPossibleReturnOfBool(fd, outp, ret, b) and
p.isBoolean(b)
// Case: a function like "if someBarrierGuard(arg) { return true } else { return false }"
(
exists(ControlFlow::ConditionGuardNode guard |
guards(guard, arg) and
guard.dominates(ret.getBasicBlock())
|
exists(boolean b |
onlyPossibleReturnOfBool(fd, outp, ret, b) and
p.isBoolean(b)
)
or
onlyPossibleReturnOfNonNil(fd, outp, ret) and
p.isNonNil()
or
onlyPossibleReturnOfNil(fd, outp, ret) and
p.isNil()
)
or
// Case: a function like "return someBarrierGuard(arg)"
// or "return !someBarrierGuard(arg) && otherCond(...)"
exists(boolean outcome |
not exists(DataFlow::Node otherRet | otherRet = outp.getEntryNode(fd) | otherRet != ret) and
this.checks(arg.asExpr(), outcome) and
// This predicate's contract is (p holds of ret ==> arg is checked),
// (and we have (this has outcome ==> arg is checked))
// but p.checkOn(ret, outcome, this) gives us (ret has outcome ==> p holds of this),
// so we need to swap outcome and (specifically boolean) p:
DataFlow::booleanProperty(outcome).checkOn(ret, p.asBoolean(), this)
)
or
// Case: a function like "return guardProxy(arg)"
// or "return !guardProxy(arg) || otherCond(...)"
exists(
Function f2, FunctionInput inp2, FunctionOutput outp2, CallNode c,
DataFlow::Property outpProp
|
not exists(DataFlow::Node otherRet | otherRet = outp.getEntryNode(fd) | otherRet != ret) and
guardingFunction(f2, inp2, outp2, outpProp) and
c = f2.getACall() and
arg = inp2.getNode(c) and
(
// See comment above ("This method's contract...") for rationale re: the inversion of
// `p` and `outpProp` here:
outpProp.checkOn(ret, p.asBoolean(), outp2.getNode(c))
or
// The particular case where p is non-boolean (i.e., nil or non-nil), and we directly return `c`:
outpProp = p and ret = outp2.getNode(c)
)
)
)
or
onlyPossibleReturnOfNonNil(fd, outp, ret) and
p.isNonNil()
or
onlyPossibleReturnOfNil(fd, outp, ret) and
p.isNil()
)
}
}
@@ -1212,7 +1249,7 @@ private predicate onlyPossibleReturnOfNonNil(FuncDecl fd, FunctionOutput res, No
}
/**
* Holds if function `f`'s result `output`, which must be a return value, is always non-nil.
* Holds if function `f`'s result `output`, which must be a return value, cannot be nil.
*/
private predicate certainlyReturnsNonNil(Function f, FunctionOutput output) {
output.isResult(_) and
@@ -1228,7 +1265,7 @@ private predicate certainlyReturnsNonNil(Function f, FunctionOutput output) {
}
/**
* Holds if `node` is always non-nil.
* Holds if `node` cannot be `nil`.
*/
private predicate isCertainlyNotNil(DataFlow::Node node) {
node instanceof DataFlow::AddressOperationNode

View File

@@ -0,0 +1,828 @@
package main
import (
"errors"
)
// Utilities: a source, a sink and a barrier guard ("isBad"):
func source() string {
return "tainted"
}
func sink(s string) {}
func isBad(s string) bool {
return len(s)%2 == 0
}
// Candidate barrier guard functions:
// Validates when returning nil
func guard(p string) error {
if isBad(p) {
return errors.New("zip contents corrupted")
}
return nil
}
// Validates when returning false
func guardBool(p string) bool {
return isBad(p)
}
// Validates when returning false
func guardBoolStmt(p string) bool {
if isBad(p) {
return true
}
return false
}
// Validates arg2 when returning true
func juggleParams(arg1, arg2 string) bool {
return arg1 == "hello world" && !guardBool(arg2)
}
// Valid when returning true
func guardBoolNeg(p string) bool {
return !isBad(p)
}
// Valid when returning false
func guardBoolCmp(p string) bool {
return isBad(p) == true
}
// Valid when returning true
func guardBoolNegCmp(p string) bool {
return isBad(p) != true
}
// Valid when returning false
func guardBoolLOrLhs(p string) bool {
return isBad(p) || p == "hello world"
}
// Doesn't reliably validate p
func guardBoolLOrNegLhs(p string) bool {
return !isBad(p) || p == "hello world"
}
// Valid when returning false
func guardBoolLOrRhs(p string) bool {
return p == "hello world" || isBad(p)
}
// Doesn't reliably validate p
func guardBoolLOrNegRhs(p string) bool {
return p == "hello world" || !isBad(p)
}
// Doesn't reliably validate p
func guardBoolLAndLhs(p string) bool {
return isBad(p) && p != "hello world"
}
// Valid when returning true
func guardBoolLAndNegLhs(p string) bool {
return !isBad(p) && p != "hello world"
}
// Doesn't reliably validate p
func guardBoolLAndRhs(p string) bool {
return p != "hello world" && isBad(p)
}
// Valid when returning true
func guardBoolLAndNegRhs(p string) bool {
return p != "hello world" && !isBad(p)
}
// Valid when returning false
func guardBoolProxy(p string) bool {
return guardBool(p)
}
// Valid when returning true
func guardBoolNegProxy(p string) bool {
return !guardBool(p)
}
// Valid when returning false
func guardBoolCmpProxy(p string) bool {
return guardBool(p) == true
}
// Valid when returning true
func guardBoolNegCmpProxy(p string) bool {
return guardBool(p) != true
}
// Valid when returning false
func guardBoolLOrLhsProxy(p string) bool {
return guardBool(p) || p == "hello world"
}
// Doesn't reliably validate p
func guardBoolLOrNegLhsProxy(p string) bool {
return !guardBool(p) || p == "hello world"
}
// Valid when returning false
func guardBoolLOrRhsProxy(p string) bool {
return p == "hello world" || guardBool(p)
}
// Doesn't reliably validate p
func guardBoolLOrNegRhsProxy(p string) bool {
return p == "hello world" || !guardBool(p)
}
// Doesn't reliably validate p
func guardBoolLAndLhsProxy(p string) bool {
return guardBool(p) && p != "hello world"
}
// Valid when returning true
func guardBoolLAndNegLhsProxy(p string) bool {
return !guardBool(p) && p != "hello world"
}
// Doesn't reliably validate p
func guardBoolLAndRhsProxy(p string) bool {
return p != "hello world" && guardBool(p)
}
// Valid when returning true
func guardBoolLAndNegRhsProxy(p string) bool {
return p != "hello world" && !guardBool(p)
}
// Valid when returning true
func guardProxyNilToBool(p string) bool {
return guard(p) == nil
}
// Valid when returning false
func guardNeqProxyNilToBool(p string) bool {
return guard(p) != nil
}
// Valid when returning false
func guardNotEqProxyNilToBool(p string) bool {
return !(guard(p) == nil)
}
// Valid when returning false
func guardLOrLhsProxyNilToBool(p string) bool {
return guard(p) != nil || p == "hello world"
}
// Doesn't reliably validate p
func guardLOrNegLhsProxyNilToBool(p string) bool {
return guard(p) == nil || p == "hello world"
}
// Valid when returning false
func guardLOrRhsProxyNilToBool(p string) bool {
return p == "hello world" || guard(p) != nil
}
// Doesn't reliably validate p
func guardLOrNegRhsProxyNilToBool(p string) bool {
return p == "hello world" || guard(p) == nil
}
// Doesn't reliably validate p
func guardLAndLhsProxyNilToBool(p string) bool {
return guard(p) != nil && p != "hello world"
}
// Valid when returning true
func guardLAndNegLhsProxyNilToBool(p string) bool {
return guard(p) == nil && p != "hello world"
}
// Doesn't reliably validate p
func guardLAndRhsProxyNilToBool(p string) bool {
return p != "hello world" && !(guard(p) == nil)
}
// Valid when returning true
func guardLAndNegRhsProxyNilToBool(p string) bool {
return p != "hello world" && guard(p) == nil
}
type retStruct struct {
s string
}
// Valid when returning nil
func guardBoolProxyToNil(p string) *retStruct {
if guardBool(p) {
return &retStruct{"bad"}
}
return nil
}
// Valid when returning non-nil
func guardBoolNegProxyToNil(p string) *retStruct {
if !guardBool(p) {
return &retStruct{"good"}
}
return nil
}
// Valid when returning nil
func guardBoolCmpProxyToNil(p string) *retStruct {
if guardBool(p) == true {
return &retStruct{"bad"}
}
return nil
}
// Valid when returning non-nil
func guardBoolNegCmpProxyToNil(p string) *retStruct {
if guardBool(p) != true {
return &retStruct{"good"}
}
return nil
}
// Valid when returning nil
func guardBoolLOrLhsProxyToNil(p string) *retStruct {
if guardBool(p) || p == "hello world" {
return &retStruct{"bad"}
}
return nil
}
// Doesn't reliably validate p
func guardBoolLOrNegLhsProxyToNil(p string) *retStruct {
if !guardBool(p) || p == "hello world" {
return &retStruct{"inconclusive"}
}
return nil
}
// Valid when returning nil
func guardBoolLOrRhsProxyToNil(p string) *retStruct {
if p == "hello world" || guardBool(p) {
return &retStruct{"bad"}
}
return nil
}
// Doesn't reliably validate p
func guardBoolLOrNegRhsProxyToNil(p string) *retStruct {
if p == "hello world" || !guardBool(p) {
return &retStruct{"inconclusive"}
}
return nil
}
// Doesn't reliably validate p
func guardBoolLAndLhsProxyToNil(p string) *retStruct {
if guardBool(p) && p != "hello world" {
return &retStruct{"inconclusive"}
}
return nil
}
// Valid when returning non-nil
func guardBoolLAndNegLhsProxyToNil(p string) *retStruct {
if !guardBool(p) && p != "hello world" {
return &retStruct{"good"}
}
return nil
}
// Doesn't reliably validate p
func guardBoolLAndRhsProxyToNil(p string) *retStruct {
if p != "hello world" && guardBool(p) {
return &retStruct{"inconclusive"}
}
return nil
}
// Valid when returning non-nil
func guardBoolLAndNegRhsProxyToNil(p string) *retStruct {
if p != "hello world" && !guardBool(p) {
return &retStruct{"good"}
}
return nil
}
// Valid when returning nil
func directProxyNil(p string) error {
return guard(p)
}
// Valid when returning true
func deeplyNestedConditionalLeft(p string) bool {
return !isBad(p) && len(p)%2 == 1 && p[0] == 'a' && p[1] == 'b'
}
// Valid when returning true
func deeplyNestedConditionalMiddle(p string) bool {
return len(p)%2 == 1 && p[0] == 'a' && !isBad(p) && p[1] == 'b'
}
// Valid when returning true
func deeplyNestedConditionalRight(p string) bool {
return p[1] == 'b' && len(p)%2 == 1 && p[0] == 'a' && !isBad(p)
}
// Finally, actually test the functions -- try sinking a tainted value in the is-true/false
// or is-nil/non-nil case for each candidate:
func test() {
{
s := source()
if guardBool(s) {
sink(s) // $dataflow=s
} else {
sink(s)
}
}
{
s := source()
if guardBoolStmt(s) {
sink(s) // $dataflow=s
} else {
sink(s)
}
}
{
s := source()
if juggleParams("other arg", s) {
sink(s)
} else {
sink(s) // $dataflow=s
}
}
{
s := source()
if guardBoolNeg(s) {
sink(s)
} else {
sink(s) // $dataflow=s
}
}
{
s := source()
if guardBoolCmp(s) {
sink(s) // $dataflow=s
} else {
sink(s)
}
}
{
s := source()
if guardBoolNegCmp(s) {
sink(s)
} else {
sink(s) // $dataflow=s
}
}
{
s := source()
if guardBoolLOrLhs(s) {
sink(s) // $dataflow=s
} else {
sink(s)
}
}
{
s := source()
if guardBoolLOrNegLhs(s) {
sink(s) // $dataflow=s
} else {
sink(s) // $dataflow=s
}
}
{
s := source()
if guardBoolLOrRhs(s) {
sink(s) // $dataflow=s
} else {
sink(s)
}
}
{
s := source()
if guardBoolLOrNegRhs(s) {
sink(s) // $dataflow=s
} else {
sink(s) // $dataflow=s
}
}
{
s := source()
if guardBoolLAndLhs(s) {
sink(s) // $dataflow=s
} else {
sink(s) // $dataflow=s
}
}
{
s := source()
if guardBoolLAndNegLhs(s) {
sink(s)
} else {
sink(s) // $dataflow=s
}
}
{
s := source()
if guardBoolLAndRhs(s) {
sink(s) // $dataflow=s
} else {
sink(s) // $dataflow=s
}
}
{
s := source()
if guardBoolLAndNegRhs(s) {
sink(s)
} else {
sink(s) // $dataflow=s
}
}
{
s := source()
if guardBoolProxy(s) {
sink(s) // $dataflow=s
} else {
sink(s)
}
}
{
s := source()
if guardBoolNegProxy(s) {
sink(s)
} else {
sink(s) // $dataflow=s
}
}
{
s := source()
if guardBoolCmpProxy(s) {
sink(s) // $dataflow=s
} else {
sink(s)
}
}
{
s := source()
if guardBoolNegCmpProxy(s) {
sink(s)
} else {
sink(s) // $dataflow=s
}
}
{
s := source()
if guardBoolLOrLhsProxy(s) {
sink(s) // $dataflow=s
} else {
sink(s)
}
}
{
s := source()
if guardBoolLOrNegLhsProxy(s) {
sink(s) // $dataflow=s
} else {
sink(s) // $dataflow=s
}
}
{
s := source()
if guardBoolLOrRhsProxy(s) {
sink(s) // $dataflow=s
} else {
sink(s)
}
}
{
s := source()
if guardBoolLOrNegRhsProxy(s) {
sink(s) // $dataflow=s
} else {
sink(s) // $dataflow=s
}
}
{
s := source()
if guardBoolLAndLhsProxy(s) {
sink(s) // $dataflow=s
} else {
sink(s) // $dataflow=s
}
}
{
s := source()
if guardBoolLAndNegLhsProxy(s) {
sink(s)
} else {
sink(s) // $dataflow=s
}
}
{
s := source()
if guardBoolLAndRhsProxy(s) {
sink(s) // $dataflow=s
} else {
sink(s) // $dataflow=s
}
}
{
s := source()
if guardBoolLAndNegRhsProxy(s) {
sink(s)
} else {
sink(s) // $dataflow=s
}
}
{
s := source()
if guardProxyNilToBool(s) {
sink(s)
} else {
sink(s) // $dataflow=s
}
}
{
s := source()
if guardNeqProxyNilToBool(s) {
sink(s) // $dataflow=s
} else {
sink(s)
}
}
{
s := source()
if guardNotEqProxyNilToBool(s) {
sink(s) // $dataflow=s
} else {
sink(s)
}
}
{
s := source()
if guardLOrLhsProxyNilToBool(s) {
sink(s) // $dataflow=s
} else {
sink(s)
}
}
{
s := source()
if guardLOrNegLhsProxyNilToBool(s) {
sink(s) // $dataflow=s
} else {
sink(s) // $dataflow=s
}
}
{
s := source()
if guardLOrRhsProxyNilToBool(s) {
sink(s) // $dataflow=s
} else {
sink(s)
}
}
{
s := source()
if guardLOrNegRhsProxyNilToBool(s) {
sink(s) // $dataflow=s
} else {
sink(s) // $dataflow=s
}
}
{
s := source()
if guardLAndLhsProxyNilToBool(s) {
sink(s) // $dataflow=s
} else {
sink(s) // $dataflow=s
}
}
{
s := source()
if guardLAndNegLhsProxyNilToBool(s) {
sink(s)
} else {
sink(s) // $dataflow=s
}
}
{
s := source()
if guardLAndRhsProxyNilToBool(s) {
sink(s) // $dataflow=s
} else {
sink(s) // $dataflow=s
}
}
{
s := source()
if guardLAndNegRhsProxyNilToBool(s) {
sink(s)
} else {
sink(s) // $dataflow=s
}
}
{
s := source()
if guard(s) == nil {
sink(s)
} else {
sink(s) // $dataflow=s
}
}
{
s := source()
if guardBoolProxyToNil(s) == nil {
sink(s)
} else {
sink(s) // $dataflow=s
}
}
{
s := source()
if guardBoolNegProxyToNil(s) == nil {
sink(s) // $dataflow=s
} else {
sink(s)
}
}
{
s := source()
if guardBoolCmpProxyToNil(s) == nil {
sink(s)
} else {
sink(s) // $dataflow=s
}
}
{
s := source()
if guardBoolNegCmpProxyToNil(s) == nil {
sink(s) // $dataflow=s
} else {
sink(s)
}
}
{
s := source()
if guardBoolLOrLhsProxyToNil(s) == nil {
sink(s)
} else {
sink(s) // $dataflow=s
}
}
{
s := source()
if guardBoolLOrNegLhsProxyToNil(s) == nil {
sink(s) // $dataflow=s
} else {
sink(s) // $dataflow=s
}
}
{
s := source()
if guardBoolLOrRhsProxyToNil(s) == nil {
sink(s)
} else {
sink(s) // $dataflow=s
}
}
{
s := source()
if guardBoolLOrNegRhsProxyToNil(s) == nil {
sink(s) // $dataflow=s
} else {
sink(s) // $dataflow=s
}
}
{
s := source()
if guardBoolLAndLhsProxyToNil(s) == nil {
sink(s) // $dataflow=s
} else {
sink(s) // $dataflow=s
}
}
{
s := source()
if guardBoolLAndNegLhsProxyToNil(s) == nil {
sink(s) // $dataflow=s
} else {
sink(s)
}
}
{
s := source()
if guardBoolLAndRhsProxyToNil(s) == nil {
sink(s) // $dataflow=s
} else {
sink(s) // $dataflow=s
}
}
{
s := source()
if guardBoolLAndNegRhsProxyToNil(s) == nil {
sink(s) // $dataflow=s
} else {
sink(s)
}
}
{
s := source()
if directProxyNil(s) == nil {
sink(s)
} else {
sink(s) // $dataflow=s
}
}
{
s := source()
if deeplyNestedConditionalLeft(s) {
sink(s)
} else {
sink(s) // $dataflow=s
}
}
{
s := source()
if deeplyNestedConditionalMiddle(s) {
sink(s)
} else {
sink(s) // $dataflow=s
}
}
{
s := source()
if deeplyNestedConditionalRight(s) {
sink(s)
} else {
sink(s) // $dataflow=s
}
}
}

View File

@@ -0,0 +1,39 @@
import go
import TestUtilities.InlineExpectationsTest
class IsBad extends DataFlow::BarrierGuard, DataFlow::CallNode {
IsBad() { this.getTarget().getName() = "isBad" }
override predicate checks(Expr e, boolean branch) {
e = this.getAnArgument().asExpr() and branch = false
}
}
class TestConfig extends DataFlow::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()
}
override predicate isBarrierGuard(DataFlow::BarrierGuard bg) { bg instanceof IsBad }
}
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, _, _, _)
)
}
}