google / go-flow-levee

Apache License 2.0
194 stars 18 forks source link

Detect incorrect sanitization by value #105

Open mlevesquedion opened 4 years ago

mlevesquedion commented 4 years ago

We may wish to be able to detect incorrect uses of sanitization by value. Consider the following sanitizer:

func Sanitize(s Source) Source {
    s.Data = "<redacted>"
    return s
}

Now consider the following incorrect use of this sanitizer:

func Oops(s Source) {
    Sanitize(s)
    Sink(s)
}

The issue is rather obvious: since Sanitize receives a copy of s, the original s is not sanitized.

Although the issue is obvious, we currently do not produce a report for it. For additional context, here is the correct way to use a "value" sanitizer:

func Value(s Source) {
    san := Sanitize(s)
    Sink(san)
}

The reason the incorrect case above does not yield a report is that our checks to determine whether a source was sanitized before reaching a sink rely on domination of the sink instruction by the sanitizer instruction. This allows us to handle cases like this:

func Pointer(s Source) {
    Sanitize(&s)
    Sink(s)
}

So it seems we will need some way to treat sanitizers differently depending on whether they take their argument by pointer or by value.

mlevesquedion commented 3 years ago

I would like to qualify this example:

func Oops(s Source) {
    Sanitize(s)
    Sink(s)
}

In some cases this may in fact not be an incorrect sanitization. Indeed, if the sensitive field is a map or other pointer-like type, the sanitizer will be able to do its job because it will get a copy of the pointer.