Skip to content

Commit 01f3793

Browse files
committed
C++: Add ReadSideEffect as a possible end instruction for load chains
1 parent a4388e9 commit 01f3793

File tree

2 files changed

+28
-14
lines changed

2 files changed

+28
-14
lines changed

cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll

Lines changed: 27 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -611,19 +611,7 @@ private newtype TLoadChain =
611611
)
612612
}
613613

614-
/**
615-
* This class abstracts out the information needed to end a `LoadChain`. For now the only
616-
* implementation is `LoadChainEndInstructionLoad`, but we may need another implementation similar
617-
* to `StoreChainEndInstructionSideEffect` to handle cases like:
618-
* ```
619-
* void read_f(Inner* inner) {
620-
* sink(inner->f);
621-
* }
622-
* ...
623-
* outer.inner.f = taint();
624-
* read_f(&outer.inner);
625-
* ```
626-
*/
614+
/** This class abstracts out the information needed to end a `LoadChain`. */
627615
abstract private class LoadChainEndInstruction extends Instruction {
628616
abstract FieldAddressInstruction getFieldInstruction();
629617

@@ -643,6 +631,32 @@ private class LoadChainEndInstructionLoad extends LoadChainEndInstruction, LoadI
643631
override Instruction getReadValue() { result = getSourceValueOperand().getAnyDef() }
644632
}
645633

634+
/**
635+
* Ends a `LoadChain` with a `ReadSideEffectInstruction`. This ensures that we pop content from the
636+
* access path when passing an argument that reads a field. For example in:
637+
* ```
638+
* void read_f(Inner* inner) {
639+
* sink(inner->f);
640+
* }
641+
* ...
642+
* outer.inner.f = taint();
643+
* read_f(&outer.inner);
644+
* ```
645+
* In order to register `inner->f` as a `readStep`, the head of the access path must
646+
* be `f`, and thus reading `&outer.inner` must pop `inner` from the access path
647+
* before entering `read_f`.
648+
*/
649+
private class LoadChainInstructionSideEffect extends LoadChainEndInstruction,
650+
ReadSideEffectInstruction {
651+
FieldAddressInstruction fi;
652+
653+
LoadChainInstructionSideEffect() { fi = skipConversion*(this.getArgumentDef()) }
654+
655+
override FieldAddressInstruction getFieldInstruction() { result = fi }
656+
657+
override Instruction getReadValue() { result = getSideEffectOperand().getAnyDef() }
658+
}
659+
646660
/**
647661
* A `LoadChain` represents a series of field lookups that compute the source address of a load.
648662
* For example, given the field lookup in `f(a.b.c)`, there are two `LoadChains`s:

cpp/ql/test/library-tests/dataflow/fields/simple.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -158,7 +158,7 @@ struct Outer
158158

159159
void read_f(Inner *inner)
160160
{
161-
sink(inner->f); //$ast $f-:ir
161+
sink(inner->f); //$ast,ir
162162
}
163163

164164
void test()

0 commit comments

Comments
 (0)