mirror of
https://github.com/github/codeql.git
synced 2026-04-21 23:14:03 +02:00
Merge pull request #14798 from owen-mc/go/improve-value-flow-through-slice-exprs
Go: model value flow with array content through slice expressions
This commit is contained in:
Owen Mansel-Chan
@@ -0,0 +1,4 @@
|
||||
---
|
||||
category: fix
|
||||
---
|
||||
* A bug has been fixed that meant that value flow through a slice expression was not tracked correctly. Taint flow was tracked correctly.
|
||||
@@ -24,6 +24,13 @@ predicate containerStoreStep(Node node1, Node node2, Content c) {
|
||||
exists(Write w | w.writesElement(node2.(PostUpdateNode).getPreUpdateNode(), _, node1))
|
||||
or
|
||||
node1 = node2.(ImplicitVarargsSlice).getCallNode().getAnImplicitVarargsArgument()
|
||||
or
|
||||
// To model data flow from array elements of the base of a `SliceNode` to
|
||||
// the `SliceNode` itself, we consider there to be a read step with array
|
||||
// content from the base to the corresponding `SliceElementNode` and then
|
||||
// a store step with array content from the `SliceelementNode` to the
|
||||
// `SliceNode` itself.
|
||||
node2 = node1.(SliceElementNode).getSliceNode()
|
||||
)
|
||||
)
|
||||
or
|
||||
@@ -57,6 +64,13 @@ predicate containerReadStep(Node node1, Node node2, Content c) {
|
||||
)
|
||||
or
|
||||
node2.(RangeElementNode).getBase() = node1
|
||||
or
|
||||
// To model data flow from array elements of the base of a `SliceNode` to
|
||||
// the `SliceNode` itself, we consider there to be a read step with array
|
||||
// content from the base to the corresponding `SliceElementNode` and then
|
||||
// a store step with array content from the `SliceelementNode` to the
|
||||
// `SliceNode` itself.
|
||||
node2.(SliceElementNode).getSliceNode().getBase() = node1
|
||||
)
|
||||
or
|
||||
c instanceof CollectionContent and
|
||||
|
||||
@@ -11,6 +11,7 @@ private newtype TNode =
|
||||
MkSsaNode(SsaDefinition ssa) or
|
||||
MkGlobalFunctionNode(Function f) or
|
||||
MkImplicitVarargsSlice(CallExpr c) { c.hasImplicitVarargs() } or
|
||||
MkSliceElementNode(SliceExpr se) or
|
||||
MkFlowSummaryNode(FlowSummaryImpl::Private::SummaryNode sn)
|
||||
|
||||
/** Nodes intended for only use inside the data-flow libraries. */
|
||||
@@ -998,6 +999,37 @@ module Public {
|
||||
Node getMax() { result = DataFlow::instructionNode(insn.getMax()) }
|
||||
}
|
||||
|
||||
/**
|
||||
* A data-flow node which exists solely to model the value flow from array
|
||||
* elements of the base of a `SliceNode` to array elements of the `SliceNode`
|
||||
* itself.
|
||||
*/
|
||||
class SliceElementNode extends Node, MkSliceElementNode {
|
||||
IR::SliceInstruction si;
|
||||
|
||||
SliceElementNode() { this = MkSliceElementNode(si.getExpr()) }
|
||||
|
||||
override ControlFlow::Root getRoot() { result = this.getSliceNode().getRoot() }
|
||||
|
||||
override Type getType() {
|
||||
result = si.getResultType().(ArrayType).getElementType() or
|
||||
result = si.getResultType().(SliceType).getElementType()
|
||||
}
|
||||
|
||||
override string getNodeKind() { result = "slice element node" }
|
||||
|
||||
override string toString() { result = "slice element node" }
|
||||
|
||||
override predicate hasLocationInfo(
|
||||
string filepath, int startline, int startcolumn, int endline, int endcolumn
|
||||
) {
|
||||
si.hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
|
||||
}
|
||||
|
||||
/** Gets the `SliceNode` which this node relates to. */
|
||||
SliceNode getSliceNode() { result = DataFlow::instructionNode(si) }
|
||||
}
|
||||
|
||||
/**
|
||||
* A data-flow node corresponding to an expression with a binary operator.
|
||||
*/
|
||||
|
||||
@@ -4,7 +4,7 @@ func source() string {
|
||||
return "untrusted data"
|
||||
}
|
||||
|
||||
func sink(string) {
|
||||
func sink(any) {
|
||||
}
|
||||
|
||||
func sliceToArray(p []string) [1]string {
|
||||
@@ -15,11 +15,15 @@ func main() {
|
||||
// Test the new slice->array conversion permitted in Go 1.20
|
||||
var a [4]string
|
||||
a[0] = source()
|
||||
alias := sliceToArray(a[:])
|
||||
sink(alias[0]) // $ hasTaintFlow="index expression"
|
||||
alias := [2]string(a[:])
|
||||
sink(alias[0]) // $ hasValueFlow="index expression"
|
||||
sink(alias[1]) // $ SPURIOUS: hasValueFlow="index expression" // we don't distinguish different elements of arrays or slices
|
||||
sink(alias) // $ hasTaintFlow="alias"
|
||||
|
||||
// Compare with the standard dataflow support for arrays
|
||||
var b [4]string
|
||||
b[0] = source()
|
||||
sink(b[0]) // $ hasValueFlow="index expression"
|
||||
sink(b[1]) // $ SPURIOUS: hasValueFlow="index expression" // we don't distinguish different elements of arrays or slices
|
||||
sink(b) // $ hasTaintFlow="b"
|
||||
}
|
||||
|
||||
@@ -0,0 +1,3 @@
|
||||
import go
|
||||
import TestUtilities.InlineFlowTest
|
||||
import DefaultFlowTest
|
||||
@@ -0,0 +1,45 @@
|
||||
package main
|
||||
|
||||
func source() string {
|
||||
return "untrusted data"
|
||||
}
|
||||
|
||||
func sink(any) {
|
||||
}
|
||||
|
||||
func main() {
|
||||
}
|
||||
|
||||
// Value flow with array content through slice expressions
|
||||
|
||||
func arrayBase(base [4]string) {
|
||||
base[1] = source()
|
||||
slice := base[1:4]
|
||||
sink(slice[0]) // $ hasValueFlow="index expression"
|
||||
sink(slice[1]) // $ SPURIOUS: hasValueFlow="index expression" // we don't distinguish different elements of arrays or slices
|
||||
sink(slice) // $ hasTaintFlow="slice"
|
||||
}
|
||||
|
||||
func arrayPointerBase(base *[4]string) {
|
||||
base[1] = source()
|
||||
slice := base[1:4]
|
||||
sink(slice[0]) // $ hasValueFlow="index expression"
|
||||
sink(slice[1]) // $ SPURIOUS: hasValueFlow="index expression" // we don't distinguish different elements of arrays or slices
|
||||
sink(slice) // $ hasTaintFlow="slice"
|
||||
}
|
||||
|
||||
func sliceBase(base []string) {
|
||||
base[1] = source()
|
||||
slice := base[1:4]
|
||||
sink(slice[0]) // $ hasValueFlow="index expression"
|
||||
sink(slice[1]) // $ SPURIOUS: hasValueFlow="index expression" // we don't distinguish different elements of arrays or slices
|
||||
sink(slice) // $ hasTaintFlow="slice"
|
||||
}
|
||||
|
||||
func slicePointerBase(base *[]string) {
|
||||
(*base)[1] = source()
|
||||
slice := (*base)[1:4]
|
||||
sink(slice[0]) // $ hasValueFlow="index expression"
|
||||
sink(slice[1]) // $ SPURIOUS: hasValueFlow="index expression" // we don't distinguish different elements of arrays or slices
|
||||
sink(slice) // $ hasTaintFlow="slice"
|
||||
}
|
||||
@@ -10,11 +10,17 @@ edges
|
||||
| GitSubcommands.go:10:13:10:27 | call to Query | GitSubcommands.go:15:35:15:41 | tainted |
|
||||
| GitSubcommands.go:10:13:10:27 | call to Query | GitSubcommands.go:16:36:16:42 | tainted |
|
||||
| SanitizingDoubleDash.go:9:13:9:19 | selection of URL | SanitizingDoubleDash.go:9:13:9:27 | call to Query |
|
||||
| SanitizingDoubleDash.go:9:13:9:27 | call to Query | SanitizingDoubleDash.go:13:25:13:31 | tainted |
|
||||
| SanitizingDoubleDash.go:9:13:9:27 | call to Query | SanitizingDoubleDash.go:14:23:14:33 | slice expression |
|
||||
| SanitizingDoubleDash.go:9:13:9:27 | call to Query | SanitizingDoubleDash.go:39:31:39:37 | tainted |
|
||||
| SanitizingDoubleDash.go:9:13:9:27 | call to Query | SanitizingDoubleDash.go:53:21:53:28 | arrayLit |
|
||||
| SanitizingDoubleDash.go:9:13:9:27 | call to Query | SanitizingDoubleDash.go:68:31:68:37 | tainted |
|
||||
| SanitizingDoubleDash.go:9:13:9:27 | call to Query | SanitizingDoubleDash.go:80:23:80:29 | tainted |
|
||||
| SanitizingDoubleDash.go:13:15:13:32 | array literal [array] | SanitizingDoubleDash.go:14:23:14:30 | arrayLit [array] |
|
||||
| SanitizingDoubleDash.go:13:25:13:31 | tainted | SanitizingDoubleDash.go:13:15:13:32 | array literal [array] |
|
||||
| SanitizingDoubleDash.go:14:23:14:30 | arrayLit [array] | SanitizingDoubleDash.go:14:23:14:33 | slice element node |
|
||||
| SanitizingDoubleDash.go:14:23:14:33 | slice element node | SanitizingDoubleDash.go:14:23:14:33 | slice expression [array] |
|
||||
| SanitizingDoubleDash.go:14:23:14:33 | slice expression [array] | SanitizingDoubleDash.go:14:23:14:33 | slice expression |
|
||||
| SanitizingDoubleDash.go:39:14:39:44 | call to append | SanitizingDoubleDash.go:40:23:40:30 | arrayLit |
|
||||
| SanitizingDoubleDash.go:39:31:39:37 | tainted | SanitizingDoubleDash.go:39:14:39:44 | call to append |
|
||||
| SanitizingDoubleDash.go:53:14:53:35 | call to append | SanitizingDoubleDash.go:54:23:54:30 | arrayLit |
|
||||
@@ -24,7 +30,9 @@ edges
|
||||
| SanitizingDoubleDash.go:69:14:69:35 | call to append | SanitizingDoubleDash.go:70:23:70:30 | arrayLit |
|
||||
| SanitizingDoubleDash.go:69:21:69:28 | arrayLit | SanitizingDoubleDash.go:69:14:69:35 | call to append |
|
||||
| SanitizingDoubleDash.go:92:13:92:19 | selection of URL | SanitizingDoubleDash.go:92:13:92:27 | call to Query |
|
||||
| SanitizingDoubleDash.go:92:13:92:27 | call to Query | SanitizingDoubleDash.go:95:25:95:31 | tainted |
|
||||
| SanitizingDoubleDash.go:92:13:92:27 | call to Query | SanitizingDoubleDash.go:96:24:96:34 | slice expression |
|
||||
| SanitizingDoubleDash.go:92:13:92:27 | call to Query | SanitizingDoubleDash.go:100:31:100:37 | tainted |
|
||||
| SanitizingDoubleDash.go:92:13:92:27 | call to Query | SanitizingDoubleDash.go:101:24:101:34 | slice expression |
|
||||
| SanitizingDoubleDash.go:92:13:92:27 | call to Query | SanitizingDoubleDash.go:105:30:105:36 | tainted |
|
||||
| SanitizingDoubleDash.go:92:13:92:27 | call to Query | SanitizingDoubleDash.go:106:24:106:31 | arrayLit |
|
||||
@@ -36,6 +44,16 @@ edges
|
||||
| SanitizingDoubleDash.go:92:13:92:27 | call to Query | SanitizingDoubleDash.go:142:31:142:37 | tainted |
|
||||
| SanitizingDoubleDash.go:92:13:92:27 | call to Query | SanitizingDoubleDash.go:148:30:148:36 | tainted |
|
||||
| SanitizingDoubleDash.go:92:13:92:27 | call to Query | SanitizingDoubleDash.go:152:24:152:30 | tainted |
|
||||
| SanitizingDoubleDash.go:95:15:95:32 | array literal [array] | SanitizingDoubleDash.go:96:24:96:31 | arrayLit [array] |
|
||||
| SanitizingDoubleDash.go:95:25:95:31 | tainted | SanitizingDoubleDash.go:95:15:95:32 | array literal [array] |
|
||||
| SanitizingDoubleDash.go:96:24:96:31 | arrayLit [array] | SanitizingDoubleDash.go:96:24:96:34 | slice element node |
|
||||
| SanitizingDoubleDash.go:96:24:96:34 | slice element node | SanitizingDoubleDash.go:96:24:96:34 | slice expression [array] |
|
||||
| SanitizingDoubleDash.go:96:24:96:34 | slice expression [array] | SanitizingDoubleDash.go:96:24:96:34 | slice expression |
|
||||
| SanitizingDoubleDash.go:100:15:100:38 | array literal [array] | SanitizingDoubleDash.go:101:24:101:31 | arrayLit [array] |
|
||||
| SanitizingDoubleDash.go:100:31:100:37 | tainted | SanitizingDoubleDash.go:100:15:100:38 | array literal [array] |
|
||||
| SanitizingDoubleDash.go:101:24:101:31 | arrayLit [array] | SanitizingDoubleDash.go:101:24:101:34 | slice element node |
|
||||
| SanitizingDoubleDash.go:101:24:101:34 | slice element node | SanitizingDoubleDash.go:101:24:101:34 | slice expression [array] |
|
||||
| SanitizingDoubleDash.go:101:24:101:34 | slice expression [array] | SanitizingDoubleDash.go:101:24:101:34 | slice expression |
|
||||
| SanitizingDoubleDash.go:105:15:105:37 | slice literal [array] | SanitizingDoubleDash.go:106:24:106:31 | arrayLit |
|
||||
| SanitizingDoubleDash.go:105:30:105:36 | tainted | SanitizingDoubleDash.go:105:15:105:37 | slice literal [array] |
|
||||
| SanitizingDoubleDash.go:111:14:111:44 | call to append | SanitizingDoubleDash.go:112:24:112:31 | arrayLit |
|
||||
@@ -68,7 +86,12 @@ nodes
|
||||
| GitSubcommands.go:16:36:16:42 | tainted | semmle.label | tainted |
|
||||
| SanitizingDoubleDash.go:9:13:9:19 | selection of URL | semmle.label | selection of URL |
|
||||
| SanitizingDoubleDash.go:9:13:9:27 | call to Query | semmle.label | call to Query |
|
||||
| SanitizingDoubleDash.go:13:15:13:32 | array literal [array] | semmle.label | array literal [array] |
|
||||
| SanitizingDoubleDash.go:13:25:13:31 | tainted | semmle.label | tainted |
|
||||
| SanitizingDoubleDash.go:14:23:14:30 | arrayLit [array] | semmle.label | arrayLit [array] |
|
||||
| SanitizingDoubleDash.go:14:23:14:33 | slice element node | semmle.label | slice element node |
|
||||
| SanitizingDoubleDash.go:14:23:14:33 | slice expression | semmle.label | slice expression |
|
||||
| SanitizingDoubleDash.go:14:23:14:33 | slice expression [array] | semmle.label | slice expression [array] |
|
||||
| SanitizingDoubleDash.go:39:14:39:44 | call to append | semmle.label | call to append |
|
||||
| SanitizingDoubleDash.go:39:31:39:37 | tainted | semmle.label | tainted |
|
||||
| SanitizingDoubleDash.go:40:23:40:30 | arrayLit | semmle.label | arrayLit |
|
||||
@@ -83,8 +106,18 @@ nodes
|
||||
| SanitizingDoubleDash.go:80:23:80:29 | tainted | semmle.label | tainted |
|
||||
| SanitizingDoubleDash.go:92:13:92:19 | selection of URL | semmle.label | selection of URL |
|
||||
| SanitizingDoubleDash.go:92:13:92:27 | call to Query | semmle.label | call to Query |
|
||||
| SanitizingDoubleDash.go:95:15:95:32 | array literal [array] | semmle.label | array literal [array] |
|
||||
| SanitizingDoubleDash.go:95:25:95:31 | tainted | semmle.label | tainted |
|
||||
| SanitizingDoubleDash.go:96:24:96:31 | arrayLit [array] | semmle.label | arrayLit [array] |
|
||||
| SanitizingDoubleDash.go:96:24:96:34 | slice element node | semmle.label | slice element node |
|
||||
| SanitizingDoubleDash.go:96:24:96:34 | slice expression | semmle.label | slice expression |
|
||||
| SanitizingDoubleDash.go:96:24:96:34 | slice expression [array] | semmle.label | slice expression [array] |
|
||||
| SanitizingDoubleDash.go:100:15:100:38 | array literal [array] | semmle.label | array literal [array] |
|
||||
| SanitizingDoubleDash.go:100:31:100:37 | tainted | semmle.label | tainted |
|
||||
| SanitizingDoubleDash.go:101:24:101:31 | arrayLit [array] | semmle.label | arrayLit [array] |
|
||||
| SanitizingDoubleDash.go:101:24:101:34 | slice element node | semmle.label | slice element node |
|
||||
| SanitizingDoubleDash.go:101:24:101:34 | slice expression | semmle.label | slice expression |
|
||||
| SanitizingDoubleDash.go:101:24:101:34 | slice expression [array] | semmle.label | slice expression [array] |
|
||||
| SanitizingDoubleDash.go:105:15:105:37 | slice literal [array] | semmle.label | slice literal [array] |
|
||||
| SanitizingDoubleDash.go:105:30:105:36 | tainted | semmle.label | tainted |
|
||||
| SanitizingDoubleDash.go:106:24:106:31 | arrayLit | semmle.label | arrayLit |
|
||||
|
||||
Reference in New Issue
Block a user