Skip to content

Commit e776178

Browse files
committed
C++: Add some whitespace to make stuff appear in the diff.
1 parent 0a41acc commit e776178

File tree

2 files changed

+167
-167
lines changed

2 files changed

+167
-167
lines changed
Lines changed: 25 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,31 +1,31 @@
1-
<!DOCTYPE qhelp PUBLIC
2-
"-//Semmle//qhelp//EN"
3-
"qhelp.dtd">
4-
<qhelp>
5-
<overview>
6-
<p>The program performs an out-of-bounds read or write operation. In addition to causing program instability, techniques exist which may allow an attacker to use this vulnerability to execute arbitrary code.</p>
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
<overview>
6+
<p>The program performs an out-of-bounds read or write operation. In addition to causing program instability, techniques exist which may allow an attacker to use this vulnerability to execute arbitrary code.</p>
77

8-
</overview>
9-
<recommendation>
8+
</overview>
9+
<recommendation>
1010

11-
<p>Ensure that pointer dereferences are properly guarded to ensure that they cannot be used to read or write past the end of the allocation.</p>
11+
<p>Ensure that pointer dereferences are properly guarded to ensure that they cannot be used to read or write past the end of the allocation.</p>
1212

13-
</recommendation>
14-
<example>
15-
<p>The first example allocates a buffer of size <code>size</code> and creates a local variable that stores the location that is one byte past the end of the allocation.
16-
This local variable is then dereferenced which results in an out-of-bounds write.
17-
The second example subtracts one from the <code>end</code> variable before dereferencing it. This subtraction ensures that the write correctly updates the final byte of the allocation.</p>
18-
<sample src="InvalidPointerDeref.cpp" />
13+
</recommendation>
14+
<example>
15+
<p>The first example allocates a buffer of size <code>size</code> and creates a local variable that stores the location that is one byte past the end of the allocation.
16+
This local variable is then dereferenced which results in an out-of-bounds write.
17+
The second example subtracts one from the <code>end</code> variable before dereferencing it. This subtraction ensures that the write correctly updates the final byte of the allocation.</p>
18+
<sample src="InvalidPointerDeref.cpp" />
1919

20-
</example>
21-
<references>
20+
</example>
21+
<references>
2222

23-
<li>CERT C Coding Standard:
24-
<a href="https://wiki.sei.cmu.edu/confluence/display/c/ARR30-C.+Do+not+form+or+use+out-of-bounds+pointers+or+array+subscripts">ARR30-C. Do not form or use out-of-bounds pointers or array subscripts</a>.</li>
25-
<li>
26-
OWASP:
27-
<a href="https://owasp.org/www-community/vulnerabilities/Buffer_Overflow">Buffer Overflow</a>.
28-
</li>
23+
<li>CERT C Coding Standard:
24+
<a href="https://wiki.sei.cmu.edu/confluence/display/c/ARR30-C.+Do+not+form+or+use+out-of-bounds+pointers+or+array+subscripts">ARR30-C. Do not form or use out-of-bounds pointers or array subscripts</a>.</li>
25+
<li>
26+
OWASP:
27+
<a href="https://owasp.org/www-community/vulnerabilities/Buffer_Overflow">Buffer Overflow</a>.
28+
</li>
2929

30-
</references>
31-
</qhelp>
30+
</references>
31+
</qhelp>
Lines changed: 142 additions & 142 deletions
Original file line numberDiff line numberDiff line change
@@ -1,153 +1,153 @@
1-
/**
2-
* @name Invalid pointer dereference
3-
* @description Dereferencing a pointer that points past it allocation is undefined behavior
4-
* and may lead to security vulnerabilities.
5-
* @kind path-problem
6-
* @problem.severity error
7-
* @security-severity 9.3
8-
* @precision medium
9-
* @id cpp/invalid-pointer-deref
10-
* @tags reliability
11-
* security
12-
* experimental
13-
* external/cwe/cwe-119
14-
* external/cwe/cwe-125
15-
* external/cwe/cwe-193
16-
* external/cwe/cwe-787
17-
*/
1+
/**
2+
* @name Invalid pointer dereference
3+
* @description Dereferencing a pointer that points past it allocation is undefined behavior
4+
* and may lead to security vulnerabilities.
5+
* @kind path-problem
6+
* @problem.severity error
7+
* @security-severity 9.3
8+
* @precision medium
9+
* @id cpp/invalid-pointer-deref
10+
* @tags reliability
11+
* security
12+
* experimental
13+
* external/cwe/cwe-119
14+
* external/cwe/cwe-125
15+
* external/cwe/cwe-193
16+
* external/cwe/cwe-787
17+
*/
1818

19-
/*
20-
* High-level description of the query:
21-
*
22-
* The goal of this query is to identify issues such as:
23-
* ```cpp
24-
* 1. int* base = new int[size];
25-
* 2. int* end = base + size;
26-
* 3. for(int* p = base; p <= end; ++p) {
27-
* 4. *p = 0; // BUG: Should have been bounded by `p < end`.
28-
* 5. }
29-
* ```
30-
* In order to do this, we split the problem into three subtasks:
31-
* 1. First, we find flow from `new int[size]` to `base + size`.
32-
* 2. Then, we find flow from `base + size` to `end` (on line 3).
33-
* 3. Finally, we use range-analysis to find a write to (or read from) a pointer that may be greater than or equal to `end`.
34-
*
35-
* Step 1 is implemented in `AllocationToInvalidPointer.qll`, and step 2 is implemented by
36-
* `InvalidPointerToDereference.qll`. See those files for the description of these.
37-
*
38-
* This file imports both libraries and defines a final dataflow configuration that constructs the full path from
39-
* the allocation to the dereference of the out-of-bounds pointer. This is done for several reasons:
40-
* 1. It means the user is able to inspect the entire path from the allocation to the dereference, which can be useful
41-
* to understand the problem highlighted.
42-
* 2. It ensures that the call-contexts line up correctly when we transition from step 1 to step 2. See the
43-
* `test_missing_call_context_1` and `test_missing_call_context_2` tests for how this may flag false positives
44-
* without this final configuration.
45-
*
46-
* The source of the final path is an allocation that is:
47-
* 1. identified as flowing to an invalid pointer (by `AllocationToInvalidPointer`), and
48-
* 2. for which the invalid pointer flows to a dereference (as identified by `InvalidPointerToDereference`).
49-
*
50-
* The path can be described in 3 "chunks":
51-
* 1. One path from the allocation to the construction of the invalid pointer
52-
* 2. Another path from the construction of the invalid pointer to the final pointer that is about to be dereferenced.
53-
* 3. Finally, a single step from the dataflow node that represents the final pointer to the dereference.
54-
*
55-
* Step 1 happens when the flow state is `TInitial`, and step 2 and 3 happen when the flow state is `TPointerArith(pai)`
56-
* where the pointer-arithmetic instruction `pai` tracks the instruction that generated the out-of-bounds pointer. This
57-
* instruction is used in the construction of the alert message.
58-
*
59-
* The set of pointer-arithmetic instructions that define the `TPointerArith` flow state is restricted to be the pointer-
60-
* arithmetic instructions that both receive flow from the allocation (as identified by `AllocationToInvalidPointer.qll`),
61-
* and further flow to a dereference (as identified by `InvalidPointerToDereference.qll`).
62-
*/
19+
/*
20+
* High-level description of the query:
21+
*
22+
* The goal of this query is to identify issues such as:
23+
* ```cpp
24+
* 1. int* base = new int[size];
25+
* 2. int* end = base + size;
26+
* 3. for(int* p = base; p <= end; ++p) {
27+
* 4. *p = 0; // BUG: Should have been bounded by `p < end`.
28+
* 5. }
29+
* ```
30+
* In order to do this, we split the problem into three subtasks:
31+
* 1. First, we find flow from `new int[size]` to `base + size`.
32+
* 2. Then, we find flow from `base + size` to `end` (on line 3).
33+
* 3. Finally, we use range-analysis to find a write to (or read from) a pointer that may be greater than or equal to `end`.
34+
*
35+
* Step 1 is implemented in `AllocationToInvalidPointer.qll`, and step 2 is implemented by
36+
* `InvalidPointerToDereference.qll`. See those files for the description of these.
37+
*
38+
* This file imports both libraries and defines a final dataflow configuration that constructs the full path from
39+
* the allocation to the dereference of the out-of-bounds pointer. This is done for several reasons:
40+
* 1. It means the user is able to inspect the entire path from the allocation to the dereference, which can be useful
41+
* to understand the problem highlighted.
42+
* 2. It ensures that the call-contexts line up correctly when we transition from step 1 to step 2. See the
43+
* `test_missing_call_context_1` and `test_missing_call_context_2` tests for how this may flag false positives
44+
* without this final configuration.
45+
*
46+
* The source of the final path is an allocation that is:
47+
* 1. identified as flowing to an invalid pointer (by `AllocationToInvalidPointer`), and
48+
* 2. for which the invalid pointer flows to a dereference (as identified by `InvalidPointerToDereference`).
49+
*
50+
* The path can be described in 3 "chunks":
51+
* 1. One path from the allocation to the construction of the invalid pointer
52+
* 2. Another path from the construction of the invalid pointer to the final pointer that is about to be dereferenced.
53+
* 3. Finally, a single step from the dataflow node that represents the final pointer to the dereference.
54+
*
55+
* Step 1 happens when the flow state is `TInitial`, and step 2 and 3 happen when the flow state is `TPointerArith(pai)`
56+
* where the pointer-arithmetic instruction `pai` tracks the instruction that generated the out-of-bounds pointer. This
57+
* instruction is used in the construction of the alert message.
58+
*
59+
* The set of pointer-arithmetic instructions that define the `TPointerArith` flow state is restricted to be the pointer-
60+
* arithmetic instructions that both receive flow from the allocation (as identified by `AllocationToInvalidPointer.qll`),
61+
* and further flow to a dereference (as identified by `InvalidPointerToDereference.qll`).
62+
*/
6363

64-
import cpp
65-
import semmle.code.cpp.dataflow.new.DataFlow
66-
import semmle.code.cpp.ir.IR
67-
import FinalFlow::PathGraph
68-
import semmle.code.cpp.security.InvalidPointerDereference.AllocationToInvalidPointer
69-
import semmle.code.cpp.security.InvalidPointerDereference.InvalidPointerToDereference
64+
import cpp
65+
import semmle.code.cpp.dataflow.new.DataFlow
66+
import semmle.code.cpp.ir.IR
67+
import FinalFlow::PathGraph
68+
import semmle.code.cpp.security.InvalidPointerDereference.AllocationToInvalidPointer
69+
import semmle.code.cpp.security.InvalidPointerDereference.InvalidPointerToDereference
7070

71-
/**
72-
* A configuration that represents the full dataflow path all the way from
73-
* the allocation to the dereference. We need this final dataflow traversal
74-
* to ensure that the transition from the sink in `AllocToInvalidPointerConfig`
75-
* to the source in `InvalidPointerToDerefFlow` did not make us construct an
76-
* infeasible path (which can happen since the transition from one configuration
77-
* to the next does not preserve information about call contexts).
78-
*/
79-
module FinalConfig implements DataFlow::StateConfigSig {
80-
newtype FlowState =
81-
additional TInitial() or
82-
additional TPointerArith(PointerArithmeticInstruction pai) {
83-
operationIsOffBy(_, pai, _, _, _, _, _)
71+
/**
72+
* A configuration that represents the full dataflow path all the way from
73+
* the allocation to the dereference. We need this final dataflow traversal
74+
* to ensure that the transition from the sink in `AllocToInvalidPointerConfig`
75+
* to the source in `InvalidPointerToDerefFlow` did not make us construct an
76+
* infeasible path (which can happen since the transition from one configuration
77+
* to the next does not preserve information about call contexts).
78+
*/
79+
module FinalConfig implements DataFlow::StateConfigSig {
80+
newtype FlowState =
81+
additional TInitial() or
82+
additional TPointerArith(PointerArithmeticInstruction pai) {
83+
operationIsOffBy(_, pai, _, _, _, _, _)
84+
}
85+
86+
predicate isSource(DataFlow::Node source, FlowState state) {
87+
state = TInitial() and
88+
operationIsOffBy(source, _, _, _, _, _, _)
8489
}
8590

86-
predicate isSource(DataFlow::Node source, FlowState state) {
87-
state = TInitial() and
88-
operationIsOffBy(source, _, _, _, _, _, _)
89-
}
91+
predicate isSink(DataFlow::Node sink, FlowState state) {
92+
exists(PointerArithmeticInstruction pai |
93+
operationIsOffBy(_, pai, _, _, _, sink, _) and
94+
state = TPointerArith(pai)
95+
)
96+
}
9097

91-
predicate isSink(DataFlow::Node sink, FlowState state) {
92-
exists(PointerArithmeticInstruction pai |
93-
operationIsOffBy(_, pai, _, _, _, sink, _) and
94-
state = TPointerArith(pai)
95-
)
98+
predicate isAdditionalFlowStep(
99+
DataFlow::Node node1, FlowState state1, DataFlow::Node node2, FlowState state2
100+
) {
101+
// A step from the left-hand side of a pointer-arithmetic operation that has been
102+
// identified as creating an out-of-bounds pointer to the result of the pointer-arithmetic
103+
// operation.
104+
exists(PointerArithmeticInstruction pai |
105+
pointerAddInstructionHasBounds(_, pai, node1, _) and
106+
operationIsOffBy(_, pai, node2, _, _, _, _) and
107+
state1 = TInitial() and
108+
state2 = TPointerArith(pai)
109+
)
110+
or
111+
// A step from an out-of-bounds address to the operation (which is either a `StoreInstruction`
112+
// or a `LoadInstruction`) that dereferences the address.
113+
// This step exists purely for aesthetic reasons: we want the alert to be placed at the operation
114+
// that causes the dereference, and not at the address that flows into the operation.
115+
state1 = state2 and
116+
exists(PointerArithmeticInstruction pai |
117+
state1 = TPointerArith(pai) and
118+
operationIsOffBy(_, pai, _, node1, _, node2, _)
119+
)
120+
}
96121
}
97122

98-
predicate isAdditionalFlowStep(
99-
DataFlow::Node node1, FlowState state1, DataFlow::Node node2, FlowState state2
123+
module FinalFlow = DataFlow::GlobalWithState<FinalConfig>;
124+
125+
/**
126+
* Holds if `source` is an allocation that flows into the left-hand side of `pai`, which produces an out-of-bounds
127+
* pointer that flows into an address that is dereferenced by `sink` (which is either a `LoadInstruction` or a
128+
* `StoreInstruction`). The end result is that `sink` writes to an address that is off-by-`delta` from the end of
129+
* the allocation. The string `operation` describes whether the `sink` is a load or a store (which is then used
130+
* to produce the alert message).
131+
*
132+
* Note that multiple `delta`s can exist for a given `(source, pai, sink)` triplet.
133+
*/
134+
predicate hasFlowPath(
135+
FinalFlow::PathNode source, FinalFlow::PathNode sink, PointerArithmeticInstruction pai,
136+
string operation, int delta
100137
) {
101-
// A step from the left-hand side of a pointer-arithmetic operation that has been
102-
// identified as creating an out-of-bounds pointer to the result of the pointer-arithmetic
103-
// operation.
104-
exists(PointerArithmeticInstruction pai |
105-
pointerAddInstructionHasBounds(_, pai, node1, _) and
106-
operationIsOffBy(_, pai, node2, _, _, _, _) and
107-
state1 = TInitial() and
108-
state2 = TPointerArith(pai)
109-
)
110-
or
111-
// A step from an out-of-bounds address to the operation (which is either a `StoreInstruction`
112-
// or a `LoadInstruction`) that dereferences the address.
113-
// This step exists purely for aesthetic reasons: we want the alert to be placed at the operation
114-
// that causes the dereference, and not at the address that flows into the operation.
115-
state1 = state2 and
116-
exists(PointerArithmeticInstruction pai |
117-
state1 = TPointerArith(pai) and
118-
operationIsOffBy(_, pai, _, node1, _, node2, _)
119-
)
138+
FinalFlow::flowPath(source, sink) and
139+
operationIsOffBy(source.getNode(), pai, _, _, operation, sink.getNode(), delta) and
140+
sink.getState() = FinalConfig::TPointerArith(pai)
120141
}
121-
}
122-
123-
module FinalFlow = DataFlow::GlobalWithState<FinalConfig>;
124-
125-
/**
126-
* Holds if `source` is an allocation that flows into the left-hand side of `pai`, which produces an out-of-bounds
127-
* pointer that flows into an address that is dereferenced by `sink` (which is either a `LoadInstruction` or a
128-
* `StoreInstruction`). The end result is that `sink` writes to an address that is off-by-`delta` from the end of
129-
* the allocation. The string `operation` describes whether the `sink` is a load or a store (which is then used
130-
* to produce the alert message).
131-
*
132-
* Note that multiple `delta`s can exist for a given `(source, pai, sink)` triplet.
133-
*/
134-
predicate hasFlowPath(
135-
FinalFlow::PathNode source, FinalFlow::PathNode sink, PointerArithmeticInstruction pai,
136-
string operation, int delta
137-
) {
138-
FinalFlow::flowPath(source, sink) and
139-
operationIsOffBy(source.getNode(), pai, _, _, operation, sink.getNode(), delta) and
140-
sink.getState() = FinalConfig::TPointerArith(pai)
141-
}
142142

143-
from
144-
FinalFlow::PathNode source, FinalFlow::PathNode sink, int k, string kstr,
145-
PointerArithmeticInstruction pai, string operation, Expr offset, DataFlow::Node n
146-
where
147-
k = min(int cand | hasFlowPath(source, sink, pai, operation, cand)) and
148-
offset = pai.getRight().getUnconvertedResultExpression() and
149-
n = source.getNode() and
150-
if k = 0 then kstr = "" else kstr = " + " + k
151-
select sink.getNode(), source, sink,
152-
"This " + operation + " might be out of bounds, as the pointer might be equal to $@ + $@" + kstr +
153-
".", n, n.toString(), offset, offset.toString()
143+
from
144+
FinalFlow::PathNode source, FinalFlow::PathNode sink, int k, string kstr,
145+
PointerArithmeticInstruction pai, string operation, Expr offset, DataFlow::Node n
146+
where
147+
k = min(int cand | hasFlowPath(source, sink, pai, operation, cand)) and
148+
offset = pai.getRight().getUnconvertedResultExpression() and
149+
n = source.getNode() and
150+
if k = 0 then kstr = "" else kstr = " + " + k
151+
select sink.getNode(), source, sink,
152+
"This " + operation + " might be out of bounds, as the pointer might be equal to $@ + $@" + kstr +
153+
".", n, n.toString(), offset, offset.toString()

0 commit comments

Comments
 (0)