google / go-flow-levee

Apache License 2.0
191 stars 20 forks source link

Improve the granularity of taint propagation in collections #47

Open mlevesquedion opened 4 years ago

mlevesquedion commented 4 years ago

Currently, when a tainted value is placed in a collection, the entire collection is considered to be tainted. This is not unreasonable considering that if a collection containing a tainted value reaches a sink, the tainted value will almost certainly "reach the sink" as well, since e.g. printing a collection implies printing its contents.

This does have the unfortunate side effect that any key access on the collection will yield a value that is considered to be tainted. For example, analyzing the following instructions yields a diagnostic, even though the value that reaches the sink is not really tainted:

m := map[string]interface{}{"source": s}
core.Sink(m[""]) // diagnostic: "a source has reached a sink"

This sequence of instructions, in which a map is properly "sanitized" (ahem...) also yields a diagnostic:

m := map[string]interface{}{"source": s}
delete(m, "source")
core.Sink(m) // diagnostic: "a source has reached a sink"

A similar "sanitization" effect could be obtained by setting a slice or map key to nil.

We would like to be able to keep track of which keys/values in a collections are actually tainted, in order to avoid producing false positives.

mlevesquedion commented 3 years ago

Some additional cases to consider:

func TestIndexByIndexSanitization(sources []core.Source) {
    sanitizedSources := make([]core.Source, len(sources))
    for i := range sources {
        sanitizedSources[i] = core.Sanitize(sources[i])[0].(core.Source)
    }
    core.Sink(sanitizedSources)
}

Index-by-index sanitization does not work in this case because sanitizedSources is considered a Source because of its type.

func TestIndexByIndexSanitizationEface(sources []core.Source) {
    sanitizedSources := make([]interface{}, len(sources))
    for i := range sources {
        sanitizedSources[i] = core.Sanitize(sources[i])[0].(core.Source)
    }
    core.Sink(sanitizedSources)
}

This case is more complicated. Among other things, it may involve #155.

Currently, one way to circumvent this is to define a sanitizer that can handle collections, and to sanitize the whole collection in one go:

func TestWholeSliceSanitization(sources []core.Source) {
    sanitizedSources := core.Sanitize(sources)
    core.Sink(sanitizedSources)
}