Skip to content

Commit 42356a8

Browse files
MathiasVPMathiasVP
authored
Merge pull request #13603 from MathiasVP/implement-is-unreachable-in-call-2
2 parents 4e4c5f8 + a4d0337 commit 42356a8

3 files changed

Lines changed: 124 additions & 10 deletions

File tree

‎cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll‎

Lines changed: 67 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -856,7 +856,73 @@ class DataFlowCall extends CallInstruction {
856856
Function getEnclosingCallable() { result = this.getEnclosingFunction() }
857857
}
858858

859-
predicate isUnreachableInCall(Node n, DataFlowCall call) { none() } // stub implementation
859+
module IsUnreachableInCall {
860+
private import semmle.code.cpp.ir.ValueNumbering
861+
private import semmle.code.cpp.controlflow.IRGuards as G
862+
863+
private class ConstantIntegralTypeArgumentNode extends PrimaryArgumentNode {
864+
int value;
865+
866+
ConstantIntegralTypeArgumentNode() {
867+
value = op.getDef().(IntegerConstantInstruction).getValue().toInt()
868+
}
869+
870+
int getValue() { result = value }
871+
}
872+
873+
pragma[nomagic]
874+
private predicate ensuresEq(Operand left, Operand right, int k, IRBlock block, boolean areEqual) {
875+
any(G::IRGuardCondition guard).ensuresEq(left, right, k, block, areEqual)
876+
}
877+
878+
pragma[nomagic]
879+
private predicate ensuresLt(Operand left, Operand right, int k, IRBlock block, boolean areEqual) {
880+
any(G::IRGuardCondition guard).ensuresLt(left, right, k, block, areEqual)
881+
}
882+
883+
predicate isUnreachableInCall(Node n, DataFlowCall call) {
884+
exists(
885+
DirectParameterNode paramNode, ConstantIntegralTypeArgumentNode arg,
886+
IntegerConstantInstruction constant, int k, Operand left, Operand right, IRBlock block
887+
|
888+
// arg flows into `paramNode`
889+
DataFlowImplCommon::viableParamArg(call, paramNode, arg) and
890+
left = constant.getAUse() and
891+
right = valueNumber(paramNode.getInstruction()).getAUse() and
892+
block = n.getBasicBlock()
893+
|
894+
// and there's a guard condition which ensures that the result of `left == right + k` is `areEqual`
895+
exists(boolean areEqual |
896+
ensuresEq(pragma[only_bind_into](left), pragma[only_bind_into](right),
897+
pragma[only_bind_into](k), pragma[only_bind_into](block), areEqual)
898+
|
899+
// this block ensures that left = right + k, but it holds that `left != right + k`
900+
areEqual = true and
901+
constant.getValue().toInt() != arg.getValue() + k
902+
or
903+
// this block ensures that or `left != right + k`, but it holds that `left = right + k`
904+
areEqual = false and
905+
constant.getValue().toInt() = arg.getValue() + k
906+
)
907+
or
908+
// or there's a guard condition which ensures that the result of `left < right + k` is `isLessThan`
909+
exists(boolean isLessThan |
910+
ensuresLt(pragma[only_bind_into](left), pragma[only_bind_into](right),
911+
pragma[only_bind_into](k), pragma[only_bind_into](block), isLessThan)
912+
|
913+
isLessThan = true and
914+
// this block ensures that `left < right + k`, but it holds that `left >= right + k`
915+
constant.getValue().toInt() >= arg.getValue() + k
916+
or
917+
// this block ensures that `left >= right + k`, but it holds that `left < right + k`
918+
isLessThan = false and
919+
constant.getValue().toInt() < arg.getValue() + k
920+
)
921+
)
922+
}
923+
}
924+
925+
import IsUnreachableInCall
860926

861927
int accessPathLimit() { result = 5 }
862928

‎cpp/ql/test/experimental/query-tests/Security/CWE/CWE-193/constant-size/ConstantSizeArrayOffByOne.expected‎

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -40,10 +40,7 @@ edges
4040
| test.cpp:159:25:159:29 | array | test.cpp:161:5:161:10 | access to array |
4141
| test.cpp:159:25:159:29 | array | test.cpp:162:5:162:10 | access to array |
4242
| test.cpp:175:30:175:30 | p | test.cpp:191:27:191:30 | access to array |
43-
| test.cpp:198:14:198:20 | buffer1 | test.cpp:175:30:175:30 | p |
44-
| test.cpp:198:14:198:20 | buffer1 | test.cpp:198:14:198:20 | buffer1 |
45-
| test.cpp:201:14:201:20 | buffer2 | test.cpp:175:30:175:30 | p |
46-
| test.cpp:201:14:201:20 | buffer2 | test.cpp:201:14:201:20 | buffer2 |
43+
| test.cpp:175:30:175:30 | p | test.cpp:191:27:191:30 | access to array |
4744
| test.cpp:204:14:204:20 | buffer3 | test.cpp:175:30:175:30 | p |
4845
| test.cpp:204:14:204:20 | buffer3 | test.cpp:204:14:204:20 | buffer3 |
4946
| test.cpp:207:35:207:35 | p | test.cpp:208:14:208:14 | p |
@@ -113,11 +110,8 @@ nodes
113110
| test.cpp:161:5:161:10 | access to array | semmle.label | access to array |
114111
| test.cpp:162:5:162:10 | access to array | semmle.label | access to array |
115112
| test.cpp:175:30:175:30 | p | semmle.label | p |
113+
| test.cpp:175:30:175:30 | p | semmle.label | p |
116114
| test.cpp:191:27:191:30 | access to array | semmle.label | access to array |
117-
| test.cpp:198:14:198:20 | buffer1 | semmle.label | buffer1 |
118-
| test.cpp:198:14:198:20 | buffer1 | semmle.label | buffer1 |
119-
| test.cpp:201:14:201:20 | buffer2 | semmle.label | buffer2 |
120-
| test.cpp:201:14:201:20 | buffer2 | semmle.label | buffer2 |
121115
| test.cpp:204:14:204:20 | buffer3 | semmle.label | buffer3 |
122116
| test.cpp:204:14:204:20 | buffer3 | semmle.label | buffer3 |
123117
| test.cpp:207:35:207:35 | p | semmle.label | p |
@@ -144,5 +138,4 @@ subpaths
144138
| test.cpp:136:9:136:16 | PointerAdd: ... += ... | test.cpp:143:18:143:21 | asdf | test.cpp:138:13:138:15 | arr | This pointer arithmetic may have an off-by-2 error allowing it to overrun $@ at this $@. | test.cpp:142:10:142:13 | asdf | asdf | test.cpp:138:12:138:15 | Load: * ... | read |
145139
| test.cpp:151:5:151:11 | PointerAdd: access to array | test.cpp:148:23:148:28 | buffer | test.cpp:151:5:151:11 | access to array | This pointer arithmetic may have an off-by-1 error allowing it to overrun $@ at this $@. | test.cpp:147:19:147:24 | buffer | buffer | test.cpp:151:5:151:15 | Store: ... = ... | write |
146140
| test.cpp:162:5:162:10 | PointerAdd: access to array | test.cpp:159:25:159:29 | array | test.cpp:162:5:162:10 | access to array | This pointer arithmetic may have an off-by-1 error allowing it to overrun $@ at this $@. | test.cpp:158:10:158:14 | array | array | test.cpp:162:5:162:19 | Store: ... = ... | write |
147-
| test.cpp:191:27:191:30 | PointerAdd: access to array | test.cpp:201:14:201:20 | buffer2 | test.cpp:191:27:191:30 | access to array | This pointer arithmetic may have an off-by-1 error allowing it to overrun $@ at this $@. | test.cpp:200:19:200:25 | buffer2 | buffer2 | test.cpp:191:27:191:30 | Load: access to array | read |
148141
| test.cpp:191:27:191:30 | PointerAdd: access to array | test.cpp:216:19:216:25 | buffer2 | test.cpp:191:27:191:30 | access to array | This pointer arithmetic may have an off-by-1 error allowing it to overrun $@ at this $@. | test.cpp:215:19:215:25 | buffer2 | buffer2 | test.cpp:191:27:191:30 | Load: access to array | read |

‎cpp/ql/test/library-tests/dataflow/dataflow-tests/test.cpp‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -734,3 +734,58 @@ void test_does_not_write_source_to_dereference()
734734
does_not_write_source_to_dereference(&x);
735735
sink(x); // $ ast,ir=733:7 SPURIOUS: ast,ir=726:11
736736
}
737+
738+
void sometimes_calls_sink_eq(int x, int n) {
739+
if(n == 0) {
740+
sink(x); // $ ast,ir=751:27 ast,ir=755:32 SPURIOUS: ast=749:27 ast,ir=753:32 // IR spurious results because we only have call contexts of depth 1
741+
}
742+
}
743+
744+
void call_sometimes_calls_sink_eq(int x, int n) {
745+
sometimes_calls_sink_eq(x, n);
746+
}
747+
748+
void test_sometimes_calls_sink_eq_1() {
749+
sometimes_calls_sink_eq(source(), 1);
750+
sometimes_calls_sink_eq(0, 0);
751+
sometimes_calls_sink_eq(source(), 0);
752+
753+
call_sometimes_calls_sink_eq(source(), 1);
754+
call_sometimes_calls_sink_eq(0, 0);
755+
call_sometimes_calls_sink_eq(source(), 0);
756+
}
757+
758+
void sometimes_calls_sink_lt(int x, int n) {
759+
if(n < 10) {
760+
sink(x); // $ ast,ir=771:27 ast,ir=775:32 SPURIOUS: ast=769:27 ast,ir=773:32 // IR spurious results because we only have call contexts of depth 1
761+
}
762+
}
763+
764+
void call_sometimes_calls_sink_lt(int x, int n) {
765+
sometimes_calls_sink_lt(x, n);
766+
}
767+
768+
void test_sometimes_calls_sink_lt() {
769+
sometimes_calls_sink_lt(source(), 10);
770+
sometimes_calls_sink_lt(0, 0);
771+
sometimes_calls_sink_lt(source(), 2);
772+
773+
call_sometimes_calls_sink_lt(source(), 10);
774+
call_sometimes_calls_sink_lt(0, 0);
775+
call_sometimes_calls_sink_lt(source(), 2);
776+
777+
}
778+
779+
void sometimes_calls_sink_switch(int x, int n) {
780+
switch(n) {
781+
case 0:
782+
sink(x); // $ ast,ir=790:31 SPURIOUS: ast,ir=788:31 // IR spurious results because IRGuard doesn't understand switch statements.
783+
break;
784+
}
785+
}
786+
787+
void test_sometimes_calls_sink_switch() {
788+
sometimes_calls_sink_switch(source(), 1);
789+
sometimes_calls_sink_switch(0, 0);
790+
sometimes_calls_sink_switch(source(), 0);
791+
}

0 commit comments

Comments
 (0)