Skip to content

C++: Fix join order problem in TaintedAllocationSize #18564

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
jketema wants to merge 1 commit into from
+15 −9
Closed

C++: Fix join order problem in TaintedAllocationSize #18564

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion cpp/ql/lib/semmle/code/cpp/controlflow/IRGuards.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -685,6 +685,12 @@ class IRGuardCondition extends Instruction {
unary_compares_eq(valueNumber(this), op, k, areEqual, value)
}

bindingset[value]
pragma[inline_late]
private predicate ensuresEqvalueControls(IRBlock block, AbstractValue value) {
this.valueControls(block, value)
}

/**
* Holds if (determined by this guard) `left == right + k` must be `areEqual` in `block`.
* If `areEqual = false` then this implies `left != right + k`.
Expand All @@ -693,7 +699,7 @@ class IRGuardCondition extends Instruction {
predicate ensuresEq(Operand left, Operand right, int k, IRBlock block, boolean areEqual) {
exists(AbstractValue value |
compares_eq(valueNumber(this), left, right, k, areEqual, value) and
this.valueControls(block, value)
this.ensuresEqvalueControls(block, value)
)
}

Expand Down
16 changes: 8 additions & 8 deletions cpp/ql/src/Security/CWE/CWE-190/TaintedAllocationSize.ql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,13 @@ predicate hasUpperBoundsCheck(Variable var) {
)
}

predicate nodeIsBarrierEqualityCandidate(DataFlow::Node node, Operand access, Variable checkedVar) {
exists(Instruction instr | instr = node.asOperand().getDef() |
readsVariable(instr, checkedVar) and
any(IRGuardCondition guard).ensuresEq(access, _, _, instr.getBlock(), true)
predicate nodeHasBarrierEquality(DataFlow::Node node) {
exists(Variable checkedVar, Operand access |
readsVariable(access.getDef(), checkedVar) and
exists(Instruction instr | instr = node.asOperand().getDef() |
readsVariable(pragma[only_bind_into](instr), pragma[only_bind_into](checkedVar)) and
any(IRGuardCondition guard).ensuresEq(access, _, _, instr.getBlock(), true)
)
)
}

Expand All @@ -76,10 +79,7 @@ module TaintedAllocationSizeConfig implements DataFlow::ConfigSig {
hasUpperBoundsCheck(checkedVar)
)
or
exists(Variable checkedVar, Operand access |
readsVariable(access.getDef(), checkedVar) and
nodeIsBarrierEqualityCandidate(node, access, checkedVar)
)
nodeHasBarrierEquality(node)
or
// block flow to inside of identified allocation functions (this flow leads
// to duplicate results)
Expand Down
Loading