diff --git a/cpp/ql/src/semmle/code/cpp/ir/dataflow/DefaultTaintTracking.qll b/cpp/ql/src/semmle/code/cpp/ir/dataflow/DefaultTaintTracking.qll index 0af802a255b9..8bbf2565fc7e 100644 --- a/cpp/ql/src/semmle/code/cpp/ir/dataflow/DefaultTaintTracking.qll +++ b/cpp/ql/src/semmle/code/cpp/ir/dataflow/DefaultTaintTracking.qll @@ -4,6 +4,8 @@ private import semmle.code.cpp.ir.dataflow.DataFlow private import semmle.code.cpp.ir.dataflow.DataFlow2 private import semmle.code.cpp.ir.IR private import semmle.code.cpp.ir.dataflow.internal.DataFlowDispatch as Dispatch +private import semmle.code.cpp.models.interfaces.Taint +private import semmle.code.cpp.models.interfaces.DataFlow /** * A predictable instruction is one where an external user can predict @@ -159,18 +161,79 @@ private predicate instructionTaintStep(Instruction i1, Instruction i2) { // This is part of the translation of `a[i]`, where we want taint to flow // from `a`. i2.(PointerAddInstruction).getLeft() = i1 - // TODO: robust Chi handling - // - // TODO: Flow from argument to return of known functions: Port missing parts - // of `returnArgument` to the `interfaces.Taint` and `interfaces.DataFlow` - // libraries. - // - // TODO: Flow from input argument to output argument of known functions: Port - // missing parts of `copyValueBetweenArguments` to the `interfaces.Taint` and - // `interfaces.DataFlow` libraries and implement call side-effect nodes. This - // will help with the test for `ExecTainted.ql`. The test for - // `TaintedPath.ql` is more tricky because the output arg is a pointer - // addition expression. + or + // Flow from argument to return value + i2 = any(CallInstruction call | + exists(int indexIn | + modelTaintToReturnValue(call.getStaticCallTarget(), indexIn) and + i1 = getACallArgumentOrIndirection(call, indexIn) + ) + ) + or + // Flow from input argument to output argument + // TODO: This won't work in practice as long as all aliased memory is tracked + // together in a single virtual variable. + // TODO: Will this work on the test for `TaintedPath.ql`, where the output arg + // is a pointer addition expression? + i2 = any(WriteSideEffectInstruction outNode | + exists(CallInstruction call, int indexIn, int indexOut | + modelTaintToParameter(call.getStaticCallTarget(), indexIn, indexOut) and + i1 = getACallArgumentOrIndirection(call, indexIn) and + outNode.getIndex() = indexOut and + outNode.getPrimaryInstruction() = call + ) + ) +} + +/** + * Get an instruction that goes into argument `argumentIndex` of `call`. This + * can be either directly or through one pointer indirection. + */ +private Instruction getACallArgumentOrIndirection(CallInstruction call, int argumentIndex) { + result = call.getPositionalArgument(argumentIndex) + or + exists(ReadSideEffectInstruction readSE | + // TODO: why are read side effect operands imprecise? + result = readSE.getSideEffectOperand().getAnyDef() and + readSE.getPrimaryInstruction() = call and + readSE.getIndex() = argumentIndex + ) +} + +private predicate modelTaintToParameter(Function f, int parameterIn, int parameterOut) { + exists(FunctionInput modelIn, FunctionOutput modelOut | + f.(TaintFunction).hasTaintFlow(modelIn, modelOut) and + (modelIn.isParameter(parameterIn) or modelIn.isParameterDeref(parameterIn)) and + modelOut.isParameterDeref(parameterOut) + ) +} + +private predicate modelTaintToReturnValue(Function f, int parameterIn) { + // Taint flow from parameter to return value + exists(FunctionInput modelIn, FunctionOutput modelOut | + f.(TaintFunction).hasTaintFlow(modelIn, modelOut) and + (modelIn.isParameter(parameterIn) or modelIn.isParameterDeref(parameterIn)) and + (modelOut.isReturnValue() or modelOut.isReturnValueDeref()) + ) + or + // Data flow (not taint flow) to where the return value points. For the time + // being we will conflate pointers and objects in taint tracking. + exists(FunctionInput modelIn, FunctionOutput modelOut | + f.(DataFlowFunction).hasDataFlow(modelIn, modelOut) and + (modelIn.isParameter(parameterIn) or modelIn.isParameterDeref(parameterIn)) and + modelOut.isReturnValueDeref() + ) + or + // Taint flow from one argument to another and data flow from an argument to a + // return value. This happens in functions like `strcat` and `memcpy`. We + // could model this flow in two separate steps, but that would add reverse + // flow from the write side-effect to the call instruction, which may not be + // desirable. + exists(int parameterMid, InParameter modelMid, OutReturnValue returnOut | + modelTaintToParameter(f, parameterIn, parameterMid) and + modelMid.isParameter(parameterMid) and + f.(DataFlowFunction).hasDataFlow(modelMid, returnOut) + ) } private Element adjustedSink(DataFlow::Node sink) { diff --git a/cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/Instruction.qll b/cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/Instruction.qll index a370888c8955..7514f580813e 100644 --- a/cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/Instruction.qll +++ b/cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/Instruction.qll @@ -1198,52 +1198,63 @@ class CallSideEffectInstruction extends SideEffectInstruction { } /** - * An instruction representing the side effect of a function call on any memory that might be read - * by that call. + * An instruction representing the side effect of a function call on any memory + * that might be read by that call. This instruction is emitted instead of + * `CallSideEffectInstruction` when it's certain that the call target cannot + * write to escaped memory. */ class CallReadSideEffectInstruction extends SideEffectInstruction { CallReadSideEffectInstruction() { getOpcode() instanceof Opcode::CallReadSideEffect } } /** - * An instruction representing the read of an indirect parameter within a function call. + * An instruction representing a read side effect of a function call on a + * specific parameter. */ -class IndirectReadSideEffectInstruction extends SideEffectInstruction { - IndirectReadSideEffectInstruction() { getOpcode() instanceof Opcode::IndirectReadSideEffect } +class ReadSideEffectInstruction extends SideEffectInstruction, IndexedInstruction { + ReadSideEffectInstruction() { getOpcode() instanceof ReadSideEffectOpcode } - Instruction getArgumentDef() { result = getAnOperand().(AddressOperand).getDef() } + /** Gets the operand for the value that will be read from this instruction, if known. */ + final SideEffectOperand getSideEffectOperand() { result = getAnOperand() } + + /** Gets the value that will be read from this instruction, if known. */ + final Instruction getSideEffect() { result = getSideEffectOperand().getDef() } + + /** Gets the operand for the address from which this instruction may read. */ + final AddressOperand getArgumentOperand() { result = getAnOperand() } - Instruction getSideEffect() { result = getAnOperand().(SideEffectOperand).getDef() } + /** Gets the address from which this instruction may read. */ + final Instruction getArgumentDef() { result = getArgumentOperand().getDef() } +} + +/** + * An instruction representing the read of an indirect parameter within a function call. + */ +class IndirectReadSideEffectInstruction extends ReadSideEffectInstruction { + IndirectReadSideEffectInstruction() { getOpcode() instanceof Opcode::IndirectReadSideEffect } } /** * An instruction representing the read of an indirect buffer parameter within a function call. */ -class BufferReadSideEffectInstruction extends SideEffectInstruction { +class BufferReadSideEffectInstruction extends ReadSideEffectInstruction { BufferReadSideEffectInstruction() { getOpcode() instanceof Opcode::BufferReadSideEffect } - - Instruction getArgumentDef() { result = getAnOperand().(AddressOperand).getDef() } - - Instruction getSideEffect() { result = getAnOperand().(SideEffectOperand).getDef() } } /** * An instruction representing the read of an indirect buffer parameter within a function call. */ -class SizedBufferReadSideEffectInstruction extends SideEffectInstruction { +class SizedBufferReadSideEffectInstruction extends ReadSideEffectInstruction { SizedBufferReadSideEffectInstruction() { getOpcode() instanceof Opcode::SizedBufferReadSideEffect } - Instruction getArgumentDef() { result = getAnOperand().(AddressOperand).getDef() } - Instruction getSizeDef() { result = getAnOperand().(BufferSizeOperand).getDef() } - - Instruction getSideEffect() { result = getAnOperand().(SideEffectOperand).getDef() } } /** - * An instruction representing a side effect of a function call. + * An instruction representing a write side effect of a function call on a + * specific parameter. */ class WriteSideEffectInstruction extends SideEffectInstruction, IndexedInstruction { WriteSideEffectInstruction() { getOpcode() instanceof WriteSideEffectOpcode } diff --git a/cpp/ql/src/semmle/code/cpp/ir/implementation/raw/Instruction.qll b/cpp/ql/src/semmle/code/cpp/ir/implementation/raw/Instruction.qll index a370888c8955..7514f580813e 100644 --- a/cpp/ql/src/semmle/code/cpp/ir/implementation/raw/Instruction.qll +++ b/cpp/ql/src/semmle/code/cpp/ir/implementation/raw/Instruction.qll @@ -1198,52 +1198,63 @@ class CallSideEffectInstruction extends SideEffectInstruction { } /** - * An instruction representing the side effect of a function call on any memory that might be read - * by that call. + * An instruction representing the side effect of a function call on any memory + * that might be read by that call. This instruction is emitted instead of + * `CallSideEffectInstruction` when it's certain that the call target cannot + * write to escaped memory. */ class CallReadSideEffectInstruction extends SideEffectInstruction { CallReadSideEffectInstruction() { getOpcode() instanceof Opcode::CallReadSideEffect } } /** - * An instruction representing the read of an indirect parameter within a function call. + * An instruction representing a read side effect of a function call on a + * specific parameter. */ -class IndirectReadSideEffectInstruction extends SideEffectInstruction { - IndirectReadSideEffectInstruction() { getOpcode() instanceof Opcode::IndirectReadSideEffect } +class ReadSideEffectInstruction extends SideEffectInstruction, IndexedInstruction { + ReadSideEffectInstruction() { getOpcode() instanceof ReadSideEffectOpcode } - Instruction getArgumentDef() { result = getAnOperand().(AddressOperand).getDef() } + /** Gets the operand for the value that will be read from this instruction, if known. */ + final SideEffectOperand getSideEffectOperand() { result = getAnOperand() } + + /** Gets the value that will be read from this instruction, if known. */ + final Instruction getSideEffect() { result = getSideEffectOperand().getDef() } + + /** Gets the operand for the address from which this instruction may read. */ + final AddressOperand getArgumentOperand() { result = getAnOperand() } - Instruction getSideEffect() { result = getAnOperand().(SideEffectOperand).getDef() } + /** Gets the address from which this instruction may read. */ + final Instruction getArgumentDef() { result = getArgumentOperand().getDef() } +} + +/** + * An instruction representing the read of an indirect parameter within a function call. + */ +class IndirectReadSideEffectInstruction extends ReadSideEffectInstruction { + IndirectReadSideEffectInstruction() { getOpcode() instanceof Opcode::IndirectReadSideEffect } } /** * An instruction representing the read of an indirect buffer parameter within a function call. */ -class BufferReadSideEffectInstruction extends SideEffectInstruction { +class BufferReadSideEffectInstruction extends ReadSideEffectInstruction { BufferReadSideEffectInstruction() { getOpcode() instanceof Opcode::BufferReadSideEffect } - - Instruction getArgumentDef() { result = getAnOperand().(AddressOperand).getDef() } - - Instruction getSideEffect() { result = getAnOperand().(SideEffectOperand).getDef() } } /** * An instruction representing the read of an indirect buffer parameter within a function call. */ -class SizedBufferReadSideEffectInstruction extends SideEffectInstruction { +class SizedBufferReadSideEffectInstruction extends ReadSideEffectInstruction { SizedBufferReadSideEffectInstruction() { getOpcode() instanceof Opcode::SizedBufferReadSideEffect } - Instruction getArgumentDef() { result = getAnOperand().(AddressOperand).getDef() } - Instruction getSizeDef() { result = getAnOperand().(BufferSizeOperand).getDef() } - - Instruction getSideEffect() { result = getAnOperand().(SideEffectOperand).getDef() } } /** - * An instruction representing a side effect of a function call. + * An instruction representing a write side effect of a function call on a + * specific parameter. */ class WriteSideEffectInstruction extends SideEffectInstruction, IndexedInstruction { WriteSideEffectInstruction() { getOpcode() instanceof WriteSideEffectOpcode } diff --git a/cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/Instruction.qll b/cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/Instruction.qll index a370888c8955..7514f580813e 100644 --- a/cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/Instruction.qll +++ b/cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/Instruction.qll @@ -1198,52 +1198,63 @@ class CallSideEffectInstruction extends SideEffectInstruction { } /** - * An instruction representing the side effect of a function call on any memory that might be read - * by that call. + * An instruction representing the side effect of a function call on any memory + * that might be read by that call. This instruction is emitted instead of + * `CallSideEffectInstruction` when it's certain that the call target cannot + * write to escaped memory. */ class CallReadSideEffectInstruction extends SideEffectInstruction { CallReadSideEffectInstruction() { getOpcode() instanceof Opcode::CallReadSideEffect } } /** - * An instruction representing the read of an indirect parameter within a function call. + * An instruction representing a read side effect of a function call on a + * specific parameter. */ -class IndirectReadSideEffectInstruction extends SideEffectInstruction { - IndirectReadSideEffectInstruction() { getOpcode() instanceof Opcode::IndirectReadSideEffect } +class ReadSideEffectInstruction extends SideEffectInstruction, IndexedInstruction { + ReadSideEffectInstruction() { getOpcode() instanceof ReadSideEffectOpcode } - Instruction getArgumentDef() { result = getAnOperand().(AddressOperand).getDef() } + /** Gets the operand for the value that will be read from this instruction, if known. */ + final SideEffectOperand getSideEffectOperand() { result = getAnOperand() } + + /** Gets the value that will be read from this instruction, if known. */ + final Instruction getSideEffect() { result = getSideEffectOperand().getDef() } + + /** Gets the operand for the address from which this instruction may read. */ + final AddressOperand getArgumentOperand() { result = getAnOperand() } - Instruction getSideEffect() { result = getAnOperand().(SideEffectOperand).getDef() } + /** Gets the address from which this instruction may read. */ + final Instruction getArgumentDef() { result = getArgumentOperand().getDef() } +} + +/** + * An instruction representing the read of an indirect parameter within a function call. + */ +class IndirectReadSideEffectInstruction extends ReadSideEffectInstruction { + IndirectReadSideEffectInstruction() { getOpcode() instanceof Opcode::IndirectReadSideEffect } } /** * An instruction representing the read of an indirect buffer parameter within a function call. */ -class BufferReadSideEffectInstruction extends SideEffectInstruction { +class BufferReadSideEffectInstruction extends ReadSideEffectInstruction { BufferReadSideEffectInstruction() { getOpcode() instanceof Opcode::BufferReadSideEffect } - - Instruction getArgumentDef() { result = getAnOperand().(AddressOperand).getDef() } - - Instruction getSideEffect() { result = getAnOperand().(SideEffectOperand).getDef() } } /** * An instruction representing the read of an indirect buffer parameter within a function call. */ -class SizedBufferReadSideEffectInstruction extends SideEffectInstruction { +class SizedBufferReadSideEffectInstruction extends ReadSideEffectInstruction { SizedBufferReadSideEffectInstruction() { getOpcode() instanceof Opcode::SizedBufferReadSideEffect } - Instruction getArgumentDef() { result = getAnOperand().(AddressOperand).getDef() } - Instruction getSizeDef() { result = getAnOperand().(BufferSizeOperand).getDef() } - - Instruction getSideEffect() { result = getAnOperand().(SideEffectOperand).getDef() } } /** - * An instruction representing a side effect of a function call. + * An instruction representing a write side effect of a function call on a + * specific parameter. */ class WriteSideEffectInstruction extends SideEffectInstruction, IndexedInstruction { WriteSideEffectInstruction() { getOpcode() instanceof WriteSideEffectOpcode } diff --git a/cpp/ql/src/semmle/code/cpp/models/implementations/Inet.qll b/cpp/ql/src/semmle/code/cpp/models/implementations/Inet.qll index 8e36ef9aa4a3..58d8b66c91a5 100644 --- a/cpp/ql/src/semmle/code/cpp/models/implementations/Inet.qll +++ b/cpp/ql/src/semmle/code/cpp/models/implementations/Inet.qll @@ -30,7 +30,7 @@ class InetAton extends TaintFunction, ArrayFunction { } } -class InetAddr extends TaintFunction, ArrayFunction { +class InetAddr extends TaintFunction, ArrayFunction, AliasFunction { InetAddr() { hasGlobalName("inet_addr") } override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) { @@ -41,6 +41,12 @@ class InetAddr extends TaintFunction, ArrayFunction { override predicate hasArrayInput(int bufParam) { bufParam = 0 } override predicate hasArrayWithNullTerminator(int bufParam) { bufParam = 0 } + + override predicate parameterNeverEscapes(int index) { index = 0 } + + override predicate parameterEscapesOnlyViaReturn(int index) { none() } + + override predicate parameterIsAlwaysReturned(int index) { none() } } class InetNetwork extends TaintFunction, ArrayFunction { diff --git a/cpp/ql/test/library-tests/dataflow/DefaultTaintTracking/defaulttainttracking.cpp b/cpp/ql/test/library-tests/dataflow/DefaultTaintTracking/defaulttainttracking.cpp new file mode 100644 index 000000000000..8958616c0d32 --- /dev/null +++ b/cpp/ql/test/library-tests/dataflow/DefaultTaintTracking/defaulttainttracking.cpp @@ -0,0 +1,41 @@ +int atoi(const char *nptr); +char *getenv(const char *name); +char *strcat(char * s1, const char * s2); + +char *strdup(const char *); +char *_strdup(const char *); +char *unmodeled_function(const char *); + +void sink(const char *); +void sink(int); + +int main(int argc, char *argv[]) { + + + + sink(_strdup(getenv("VAR"))); + sink(strdup(getenv("VAR"))); + sink(unmodeled_function(getenv("VAR"))); + + char untainted_buf[100] = ""; + char buf[100] = "VAR = "; + sink(strcat(buf, getenv("VAR"))); + + sink(buf); // BUG: no taint + sink(untainted_buf); // the two buffers would be conflated if we added flow through partial chi inputs + + return 0; +} + +typedef unsigned int inet_addr_retval; +inet_addr_retval inet_addr(const char *dotted_address); +void sink(inet_addr_retval); + +void test_indirect_arg_to_model() { + // This test is non-sensical but carefully arranged so we get data flow into + // inet_addr not through the function argument but through its associated + // read side effect. + void *env_pointer = getenv("VAR"); // env_pointer is tainted, not its data. + inet_addr_retval a = inet_addr((const char *)&env_pointer); + sink(a); +} diff --git a/cpp/ql/test/library-tests/dataflow/DefaultTaintTracking/tainted.expected b/cpp/ql/test/library-tests/dataflow/DefaultTaintTracking/tainted.expected new file mode 100644 index 000000000000..366dcf498817 --- /dev/null +++ b/cpp/ql/test/library-tests/dataflow/DefaultTaintTracking/tainted.expected @@ -0,0 +1,29 @@ +| defaulttainttracking.cpp:16:16:16:21 | call to getenv | defaulttainttracking.cpp:6:15:6:24 | p#0 | +| defaulttainttracking.cpp:16:16:16:21 | call to getenv | defaulttainttracking.cpp:9:11:9:20 | p#0 | +| defaulttainttracking.cpp:16:16:16:21 | call to getenv | defaulttainttracking.cpp:16:8:16:14 | call to _strdup | +| defaulttainttracking.cpp:16:16:16:21 | call to getenv | defaulttainttracking.cpp:16:8:16:29 | (const char *)... | +| defaulttainttracking.cpp:16:16:16:21 | call to getenv | defaulttainttracking.cpp:16:16:16:21 | call to getenv | +| defaulttainttracking.cpp:16:16:16:21 | call to getenv | defaulttainttracking.cpp:16:16:16:28 | (const char *)... | +| defaulttainttracking.cpp:17:15:17:20 | call to getenv | defaulttainttracking.cpp:5:14:5:23 | p#0 | +| defaulttainttracking.cpp:17:15:17:20 | call to getenv | defaulttainttracking.cpp:9:11:9:20 | p#0 | +| defaulttainttracking.cpp:17:15:17:20 | call to getenv | defaulttainttracking.cpp:17:8:17:13 | call to strdup | +| defaulttainttracking.cpp:17:15:17:20 | call to getenv | defaulttainttracking.cpp:17:8:17:28 | (const char *)... | +| defaulttainttracking.cpp:17:15:17:20 | call to getenv | defaulttainttracking.cpp:17:15:17:20 | call to getenv | +| defaulttainttracking.cpp:17:15:17:20 | call to getenv | defaulttainttracking.cpp:17:15:17:27 | (const char *)... | +| defaulttainttracking.cpp:18:27:18:32 | call to getenv | defaulttainttracking.cpp:7:26:7:35 | p#0 | +| defaulttainttracking.cpp:18:27:18:32 | call to getenv | defaulttainttracking.cpp:18:27:18:32 | call to getenv | +| defaulttainttracking.cpp:18:27:18:32 | call to getenv | defaulttainttracking.cpp:18:27:18:39 | (const char *)... | +| defaulttainttracking.cpp:22:20:22:25 | call to getenv | defaulttainttracking.cpp:3:38:3:39 | s2 | +| defaulttainttracking.cpp:22:20:22:25 | call to getenv | defaulttainttracking.cpp:9:11:9:20 | p#0 | +| defaulttainttracking.cpp:22:20:22:25 | call to getenv | defaulttainttracking.cpp:22:8:22:13 | call to strcat | +| defaulttainttracking.cpp:22:20:22:25 | call to getenv | defaulttainttracking.cpp:22:8:22:33 | (const char *)... | +| defaulttainttracking.cpp:22:20:22:25 | call to getenv | defaulttainttracking.cpp:22:20:22:25 | call to getenv | +| defaulttainttracking.cpp:22:20:22:25 | call to getenv | defaulttainttracking.cpp:22:20:22:32 | (const char *)... | +| defaulttainttracking.cpp:38:25:38:30 | call to getenv | defaulttainttracking.cpp:32:11:32:26 | p#0 | +| defaulttainttracking.cpp:38:25:38:30 | call to getenv | defaulttainttracking.cpp:38:11:38:21 | env_pointer | +| defaulttainttracking.cpp:38:25:38:30 | call to getenv | defaulttainttracking.cpp:38:25:38:30 | call to getenv | +| defaulttainttracking.cpp:38:25:38:30 | call to getenv | defaulttainttracking.cpp:38:25:38:37 | (void *)... | +| defaulttainttracking.cpp:38:25:38:30 | call to getenv | defaulttainttracking.cpp:39:22:39:22 | a | +| defaulttainttracking.cpp:38:25:38:30 | call to getenv | defaulttainttracking.cpp:39:26:39:34 | call to inet_addr | +| defaulttainttracking.cpp:38:25:38:30 | call to getenv | defaulttainttracking.cpp:39:50:39:61 | & ... | +| defaulttainttracking.cpp:38:25:38:30 | call to getenv | defaulttainttracking.cpp:40:10:40:10 | a | diff --git a/cpp/ql/test/library-tests/dataflow/DefaultTaintTracking/tainted.ql b/cpp/ql/test/library-tests/dataflow/DefaultTaintTracking/tainted.ql new file mode 100644 index 000000000000..bbd94cab403a --- /dev/null +++ b/cpp/ql/test/library-tests/dataflow/DefaultTaintTracking/tainted.ql @@ -0,0 +1,5 @@ +import semmle.code.cpp.ir.dataflow.DefaultTaintTracking + +from Expr source, Element tainted +where tainted(source, tainted) +select source, tainted diff --git a/csharp/ql/src/semmle/code/csharp/ir/implementation/raw/Instruction.qll b/csharp/ql/src/semmle/code/csharp/ir/implementation/raw/Instruction.qll index a370888c8955..7514f580813e 100644 --- a/csharp/ql/src/semmle/code/csharp/ir/implementation/raw/Instruction.qll +++ b/csharp/ql/src/semmle/code/csharp/ir/implementation/raw/Instruction.qll @@ -1198,52 +1198,63 @@ class CallSideEffectInstruction extends SideEffectInstruction { } /** - * An instruction representing the side effect of a function call on any memory that might be read - * by that call. + * An instruction representing the side effect of a function call on any memory + * that might be read by that call. This instruction is emitted instead of + * `CallSideEffectInstruction` when it's certain that the call target cannot + * write to escaped memory. */ class CallReadSideEffectInstruction extends SideEffectInstruction { CallReadSideEffectInstruction() { getOpcode() instanceof Opcode::CallReadSideEffect } } /** - * An instruction representing the read of an indirect parameter within a function call. + * An instruction representing a read side effect of a function call on a + * specific parameter. */ -class IndirectReadSideEffectInstruction extends SideEffectInstruction { - IndirectReadSideEffectInstruction() { getOpcode() instanceof Opcode::IndirectReadSideEffect } +class ReadSideEffectInstruction extends SideEffectInstruction, IndexedInstruction { + ReadSideEffectInstruction() { getOpcode() instanceof ReadSideEffectOpcode } - Instruction getArgumentDef() { result = getAnOperand().(AddressOperand).getDef() } + /** Gets the operand for the value that will be read from this instruction, if known. */ + final SideEffectOperand getSideEffectOperand() { result = getAnOperand() } + + /** Gets the value that will be read from this instruction, if known. */ + final Instruction getSideEffect() { result = getSideEffectOperand().getDef() } + + /** Gets the operand for the address from which this instruction may read. */ + final AddressOperand getArgumentOperand() { result = getAnOperand() } - Instruction getSideEffect() { result = getAnOperand().(SideEffectOperand).getDef() } + /** Gets the address from which this instruction may read. */ + final Instruction getArgumentDef() { result = getArgumentOperand().getDef() } +} + +/** + * An instruction representing the read of an indirect parameter within a function call. + */ +class IndirectReadSideEffectInstruction extends ReadSideEffectInstruction { + IndirectReadSideEffectInstruction() { getOpcode() instanceof Opcode::IndirectReadSideEffect } } /** * An instruction representing the read of an indirect buffer parameter within a function call. */ -class BufferReadSideEffectInstruction extends SideEffectInstruction { +class BufferReadSideEffectInstruction extends ReadSideEffectInstruction { BufferReadSideEffectInstruction() { getOpcode() instanceof Opcode::BufferReadSideEffect } - - Instruction getArgumentDef() { result = getAnOperand().(AddressOperand).getDef() } - - Instruction getSideEffect() { result = getAnOperand().(SideEffectOperand).getDef() } } /** * An instruction representing the read of an indirect buffer parameter within a function call. */ -class SizedBufferReadSideEffectInstruction extends SideEffectInstruction { +class SizedBufferReadSideEffectInstruction extends ReadSideEffectInstruction { SizedBufferReadSideEffectInstruction() { getOpcode() instanceof Opcode::SizedBufferReadSideEffect } - Instruction getArgumentDef() { result = getAnOperand().(AddressOperand).getDef() } - Instruction getSizeDef() { result = getAnOperand().(BufferSizeOperand).getDef() } - - Instruction getSideEffect() { result = getAnOperand().(SideEffectOperand).getDef() } } /** - * An instruction representing a side effect of a function call. + * An instruction representing a write side effect of a function call on a + * specific parameter. */ class WriteSideEffectInstruction extends SideEffectInstruction, IndexedInstruction { WriteSideEffectInstruction() { getOpcode() instanceof WriteSideEffectOpcode } diff --git a/csharp/ql/src/semmle/code/csharp/ir/implementation/unaliased_ssa/Instruction.qll b/csharp/ql/src/semmle/code/csharp/ir/implementation/unaliased_ssa/Instruction.qll index a370888c8955..7514f580813e 100644 --- a/csharp/ql/src/semmle/code/csharp/ir/implementation/unaliased_ssa/Instruction.qll +++ b/csharp/ql/src/semmle/code/csharp/ir/implementation/unaliased_ssa/Instruction.qll @@ -1198,52 +1198,63 @@ class CallSideEffectInstruction extends SideEffectInstruction { } /** - * An instruction representing the side effect of a function call on any memory that might be read - * by that call. + * An instruction representing the side effect of a function call on any memory + * that might be read by that call. This instruction is emitted instead of + * `CallSideEffectInstruction` when it's certain that the call target cannot + * write to escaped memory. */ class CallReadSideEffectInstruction extends SideEffectInstruction { CallReadSideEffectInstruction() { getOpcode() instanceof Opcode::CallReadSideEffect } } /** - * An instruction representing the read of an indirect parameter within a function call. + * An instruction representing a read side effect of a function call on a + * specific parameter. */ -class IndirectReadSideEffectInstruction extends SideEffectInstruction { - IndirectReadSideEffectInstruction() { getOpcode() instanceof Opcode::IndirectReadSideEffect } +class ReadSideEffectInstruction extends SideEffectInstruction, IndexedInstruction { + ReadSideEffectInstruction() { getOpcode() instanceof ReadSideEffectOpcode } - Instruction getArgumentDef() { result = getAnOperand().(AddressOperand).getDef() } + /** Gets the operand for the value that will be read from this instruction, if known. */ + final SideEffectOperand getSideEffectOperand() { result = getAnOperand() } + + /** Gets the value that will be read from this instruction, if known. */ + final Instruction getSideEffect() { result = getSideEffectOperand().getDef() } + + /** Gets the operand for the address from which this instruction may read. */ + final AddressOperand getArgumentOperand() { result = getAnOperand() } - Instruction getSideEffect() { result = getAnOperand().(SideEffectOperand).getDef() } + /** Gets the address from which this instruction may read. */ + final Instruction getArgumentDef() { result = getArgumentOperand().getDef() } +} + +/** + * An instruction representing the read of an indirect parameter within a function call. + */ +class IndirectReadSideEffectInstruction extends ReadSideEffectInstruction { + IndirectReadSideEffectInstruction() { getOpcode() instanceof Opcode::IndirectReadSideEffect } } /** * An instruction representing the read of an indirect buffer parameter within a function call. */ -class BufferReadSideEffectInstruction extends SideEffectInstruction { +class BufferReadSideEffectInstruction extends ReadSideEffectInstruction { BufferReadSideEffectInstruction() { getOpcode() instanceof Opcode::BufferReadSideEffect } - - Instruction getArgumentDef() { result = getAnOperand().(AddressOperand).getDef() } - - Instruction getSideEffect() { result = getAnOperand().(SideEffectOperand).getDef() } } /** * An instruction representing the read of an indirect buffer parameter within a function call. */ -class SizedBufferReadSideEffectInstruction extends SideEffectInstruction { +class SizedBufferReadSideEffectInstruction extends ReadSideEffectInstruction { SizedBufferReadSideEffectInstruction() { getOpcode() instanceof Opcode::SizedBufferReadSideEffect } - Instruction getArgumentDef() { result = getAnOperand().(AddressOperand).getDef() } - Instruction getSizeDef() { result = getAnOperand().(BufferSizeOperand).getDef() } - - Instruction getSideEffect() { result = getAnOperand().(SideEffectOperand).getDef() } } /** - * An instruction representing a side effect of a function call. + * An instruction representing a write side effect of a function call on a + * specific parameter. */ class WriteSideEffectInstruction extends SideEffectInstruction, IndexedInstruction { WriteSideEffectInstruction() { getOpcode() instanceof WriteSideEffectOpcode }