Fuzz test Prometheus APIs

[fabxc]

Just found this new tool for Go: https://github.com/dvyukov/go-fuzz

Given that it found quite a few bugs in the standard library, we will certainly find some stuff in Prometheus. Especially as we are now using two text/template style parsers ourselves that might suffer from very similar bugs as the ones found in the stdlib.

[beorn7]

Arguably, this is more a client_golang issue (or the upcoming expfmt), but let's leave it here for now.

[fabxc]

The query language could use a run as well ;)

On Wed, Jul 15, 2015 at 1:35 PM Björn Rabenstein notifications@github.com wrote:

Arguably, this is more a client_golang issue (or the upcoming expfmt), but let's leave it here for now.

— Reply to this email directly or view it on GitHub https://github.com/prometheus/prometheus/issues/667#issuecomment-121587617 .

[msiebuhr]

Played a bit with the go-fuzz, first implementing promql/fuzz.go, by testing all the Parser*()-functions:

package promql
// +build gofuzz

const (
    fuzz_interesting = 1
    fuzz_meh         = 0
    fuzz_discard     = -1
)

// Fuzz the promql parser
func Fuzz(input []byte) int {
    str := string(input)

    // Parse as metric
    _, err := ParseMetric(str)
    if err == nil {
        return fuzz_interesting
    }

    // What about a selector
    _, err = ParseMetricSelector(str)
    if err == nil {
        return fuzz_interesting
    }

    _, err = ParseExpr(str)
    if err == nil {
        return fuzz_interesting
    }

    _, err = ParseStmts(str)
    if err == nil {
        return fuzz_interesting
    }

    return fuzz_discard
}

I then gave it a small input corpus in ./corpus/with individual files containing name{} 42, a{b="c"} 1.0 456, metric_name and avg(metric_name{}[5m]) by (key1, key2). (I'm not certain what the parser(s) are for, so I conjured some simple inputs from memory...)

Then I ran godep restore to set the current $GOPATH up with all the right versions of relevant packages. Finally, go-fuzz-build github.com/prometheus/prometheus/promql instruments the code and go-fuzz -bin=./promql-fuzz.zip -workdir=fuzz-workdir to test it out. It crashes a lot in the beginning, but restarting it a few times seem to improve things dramatically.

I got a few hundred crashing examples from running it in half an hour or so, but it looks like the stack traces are all over the place, so I'm a bit hesitant to begin opening bug-reports left and right. Also, this doesn't keep track of which parser crashes, making triage more difficult.

Right now I'm trying to do quick runs of the fuzzer with one function at a time and see what I get.

[msiebuhr]

Hit in ParseMetricSelector(); it crashes on inputs .٩, G.٩ and .٩٩٩٩٩٩٩٩٩٩٩٩٩٩٩٩٩...

[msiebuhr]

Reported the first cases as separate bugs. One noteworthy find was that no crashes was found in ParseMetric()...

[juliusv]

@msiebuhr This is brilliant, thanks so much! This is exactly what we were hoping for (well, uh, not the crashes, they are bad, but that uncovering them works so well like this). @fabxc is on vacation this week and back on Monday, so it might take us a bit to get to them all, but we'll definitely address all of these soon!

Fuzzing results are scary...

[msiebuhr]

I'd be happy to make a pull-request w. some tidying up, documentation &c, if you're interested.

Also, go-fuzz mentions that "thousand[s] of inputs is fine", while I started out with the five above; I have a hunch that more variety would be nice.

[fabxc]

Fantastic! This is basically what I expected would happen as parser and lexer are tested against valid input and common mistakes or typos - not against the whole variety of possible Unicode strings. I hope we can isolate a few common root causes to fix the majority.

Definitely great work. A pull request would, of course, be awesome.

On Wed, Jul 29, 2015, 8:20 AM Morten Siebuhr notifications@github.com wrote:

I'd be happy to make a pull-request w. some tidying up, documentation &c, if you're interested.

Also, go-fuzz mentions that "thousand[s] of inputs is fine", while I started out with the five above; I have a hunch that more variety would be nice.

— Reply to this email directly or view it on GitHub https://github.com/prometheus/prometheus/issues/667#issuecomment-125854018 .

[msiebuhr]

I've put my work in a branch, https://github.com/msiebuhr/prometheus/tree/fuzz/promql, with a fuzzer-function for each parser + small corpuses for ParseMetric and ParseExpr:

go-fuzz-build -func FuzzParseExpr -o FuzzParseExpr.zip github.com/prometheus/prometheus/promql
go-fuzz -bin FuzzParseExpr.zip -workdir fuzz-data/ParseExpr

From my (limited) experience, the fuzzer really improves with larger corpuses. So far I've mostly copy-pasted from the test-suites. I thought about just writing out the files directly from the test-suite, but I'm getting a bit too tired to hammer that out.

I also poked a bit at setting up the Makefile, but I screwed something up somewhere and thought it would be better to polish the working parts a bit more and get it out.

[msiebuhr]

I've created an in-progress pullrequest over at #945.

[brian-brazil]

I believe we still need to make this vaguely automatic.

[brian-brazil]

Things left to fuzz:

HTTP API Remote read

[bboreham]

Discussed at bug-scrub: there are fuzz-tests for the WAL and PromQL. APIs as mentioned above, plus remote-write receiver should be added.