Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit 499ab08

Browse filesBrowse files
committed
C++: Currently, to catch flow in an example such as:
```cpp char* source(); void sink(const char*); int sprintf(char *, const char *, ...); void call_sprintf(char* path, char* data) { sprintf(path, "%s", "abc"); // (1) sprintf(path, "%s", data); // (2) } void foo() { char path[10]; call_sprintf(path, source()); // (3) sink(path); } ``` we identify that the `*path [post update]` node at `// (2)` is a `ReturnNodeExt` and since `*data` flows to that node flow will be carried out to `*path [post update]` at // (3) and thus reach `sink(path)`. The reason `*path [post update]` at `// 2` is recognized as a `ReturnNodeExt` is because it satisfies the following condition (which is identified by the shared dataflow library): There is flow from the parameter node `*path` to the pre-update node of the post-update node `*path [post update]` at `// (2)`. However, when we start recognizing that the call to `sprintf(path, ...)` at `// (1)` overrides the value of `*path` and no longer provide use-use flow out of `*path` the `*path [post update]` node at `// (2)` is no longer recognized as a `ReturnNodeExt` (because it doesn't satisfy the above criteria). Thus, we need to identify the flow above without relying on the dataflow library's summary mechanism. That is, instead of relying on the dataflow library's mechanism to summarize the `*data -> *path` flow for `call_sprintf` we need to: - Ensure that the write to `*path` at `// (2)` is recognized as the "final" write to the parameter, and - Ensure that there's flow out of that parameter and back to `*path [post update]` at `// (3)`. Luckiky, we do all of this already to support flow out of writes to parameters that don't have post-update nodes. For example, in something like: ```cpp void set(int* x, int y) { *x = y; } void test() { int x; set(&x, source()); sink(x); } ``` So in order to make the original example work, all we need to do is to remove the restrictions on this mechanism so that the same mechanism that makes the above example work also makes the original example work!
1 parent 7e9bf2a commit 499ab08
Copy full SHA for 499ab08

File tree

2 files changed

+6
-24
lines changed
Filter options

2 files changed

+6
-24
lines changed

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

Copy file name to clipboardExpand all lines: cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll
+1-18Lines changed: 1 addition & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -55,29 +55,12 @@ private newtype TIRDataFlowNode =
5555
TFinalParameterNode(Parameter p, int indirectionIndex) {
5656
exists(Ssa::FinalParameterUse use |
5757
use.getParameter() = p and
58-
use.getIndirectionIndex() = indirectionIndex and
59-
parameterIsRedefined(p)
58+
use.getIndirectionIndex() = indirectionIndex
6059
)
6160
} or
6261
TFinalGlobalValue(Ssa::GlobalUse globalUse) or
6362
TInitialGlobalValue(Ssa::GlobalDef globalUse)
6463

65-
/**
66-
* Holds if the value of `*p` (or `**p`, `***p`, etc.) is redefined somewhere in the body
67-
* of the enclosing function of `p`.
68-
*
69-
* Only parameters satisfying this predicate will generate a `FinalParameterNode` transferring
70-
* flow out of the function.
71-
*/
72-
private predicate parameterIsRedefined(Parameter p) {
73-
exists(Ssa::Def def |
74-
def.getSourceVariable().getBaseVariable().(Ssa::BaseIRVariable).getIRVariable().getAst() = p and
75-
def.getIndirectionIndex() = 0 and
76-
def.getIndirection() > 1 and
77-
not def.getValue().asInstruction() instanceof InitializeParameterInstruction
78-
)
79-
}
80-
8164
/**
8265
* An operand that is defined by a `FieldAddressInstruction`.
8366
*/

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

Copy file name to clipboardExpand all lines: cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternals.qll
+5-6Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -142,12 +142,11 @@ private newtype TDefOrUseImpl =
142142
isIteratorUse(container, iteratorAddress, _, indirectionIndex)
143143
} or
144144
TFinalParameterUse(Parameter p, int indirectionIndex) {
145-
// Avoid creating parameter nodes if there is no definitions of the variable other than the initializaion.
146-
exists(SsaInternals0::Def def |
147-
def.getSourceVariable().getBaseVariable().(BaseIRVariable).getIRVariable().getAst() = p and
148-
not def.getValue().asInstruction() instanceof InitializeParameterInstruction and
149-
underlyingTypeIsModifiableAt(p.getUnderlyingType(), indirectionIndex)
150-
)
145+
underlyingTypeIsModifiableAt(p.getUnderlyingType(), indirectionIndex) and
146+
// Only create an SSA read for the final use of a parameter if there's
147+
// actually a body of the enclosing function. If there's no function body
148+
// then we'll never need to flow out of the function anyway.
149+
p.getFunction().hasDefinition()
151150
}
152151

153152
private predicate isGlobalUse(

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.