Nine-year-old bug in the Go standard library enables DoS | Hacker News Reader

Nine-year-old bug in the Go standard library enables DoS

github.com

167 pointslatchkey30 comments

The issue:

"Certain invalid inputs to ReadUvarint or ReadVarint could cause those functions to read an unlimited number of bytes from the ByteReader argument before returning an error. This could lead to processing more input than expected when the caller is reading directly from a network and depends on ReadUvarint and ReadVarint only consuming a small, bounded number of bytes, even from invalid inputs.

"With the update, ReadUvarint and ReadVarint now always return after consuming a bounded number of bytes (specifically, MaxVarintLen64, which is 10). The result being returned has not changed; the functions merely detect and return some errors without reading as much input."

What's a varint? This: https://developers.google.com/protocol-buffers/docs/encoding

You have to use varints to be affected, but of course you could be using them in a library without knowing it.

So is it Golang’s varint or protobuf/grpc varint that had the bug? It’s not clear to me.

Edit: https://go-review.googlesource.com/c/go/+/247120/

Proto has a different structure with a fully unrolled loop. It could never read more than 10 bytes. encoding/binary just looped as long as the more bit was set, which is a hazard of more-bit encodings (which suck; unary-length-prefixed codes like UTF-8 are strictly better in every way).

Woops. Sounds like an big mistake. Fuzzing or even protest would have caught that.

I think it's unlikely this would have been caught by fuzzing. I think you'd have to write assertions that it only read as many bytes as necessary, which is basically saying just don't write the bug in the first place.

Not sure but couldn't a fuzzer have created an input that would have timed out the fuzzer? I guess it also depends on the timeout you set when running it.

You would also have needed to have a slow unboundedly large data stream on the other end. I fuzzed a bunch of varint-using code last month and never found this, because I was always working with in-memory buffers. The loop is simple enough that anything reasonably sized (e.g. under a GB) would probably not hit any reasonable timeout.

Hypothetically yes, but I'm not sure any common fuzzing strategy would catch that. Pure random mutations would never get this far, and code-coverage based mutation won't either. How to create a strategy that would catch this but not be rife with false positives and wasted fuzzing time is not immediately obvious.

Go doesn't have assertions. Of course it is not hard to mimic them, but it's an intentional choice to increase the friction of doing that and make it feel unidiomatic.

Does Go really aim to discourage the programmer from adding runtime checks? What's the reasoning there?

An assertion, I think, would crash the program. In that sense implementing them is simple - just panic when the condition is violated. Go prefers plain checks that return errors, though. Panics should be used for truly unexpected conditions, such as a nil pointer. Invalid input, as in this case, would be better served as an error.

> Panics should be used for truly unexpected conditions, such as a nil pointer.

Exceptional cases, if you will.

I find it funny when people say Go doesn't have exceptions when Go is the only mainstream language that advocates for and prefers the use of exceptions.

To encourage "proper error handling and reporting" according to the FAQ.

https://golang.org/doc/faq#assertions

Yes, Go really does discourage assertions.

I should not have used the word "assertion". I meant it in the more general way of "code that checks some property", not a literal "assert" feature as built in to the language.

I was just pointing out that this bug basically requires an explicit test checking for exactly the bug. Fuzzing would not have found the check for you.

FWIW, more bit encodings allow you to quickly "resynchronize" a stream if you are dropped in the middle... UTF-8 is actually a hybrid and uses both strategies.

Misleading title. Reads like the bug was discovered 9 years ago but was not fixed until recently. However, the bug seems to have been fixed pretty fast. Thanks to the Go team!

Another way to read it is that 9 years worth of software could be susceptible. It’s patched now but how long will it take that all executables have been updated? How long for all projects to update their dependencies to one with a fix? Generally software deployment also varies wildly in terms of quality and speed.

And, since people tend to link Go statically, it's not just like `apt-get upgrade libgo`, one has to do a full recompile. (Which, to be fair, Go makes a lot less painful than many languages.)

bugs objectively exist regardless of if they are discovered or not

For it to be a bug, it must have a context where it matters, but basically, yes :)

9 years may seem like a long time, but its also just a DoS vuln. Which is a real security issue in many contexts, but its not exactly a high severity one

It wasn't a 9 year old reported bug that took 9 years to fix. It was a 9 year old undetected bug that took... I'm not sure, it looks like a 1 week from discovery by the Ethereum team to a fixed released by the Go team.

Interesting that you and another commenter interpreted "9 year old bug" to mean "bug discovered 9 years ago". I'd have thought it was unambiguous that the bug was introduced 9 years ago, but apparently it isn't!

From the article, "this attack was due to a 9 year old bug in the Go standard library. During the "post-mortem" @protolambda, @prestonvanloon, @raulk and I uncovered this bug," so at least they are quite clear that it is a recently discovered bug.

> I'd have thought it was unambiguous that the bug was introduced 9 years ago, but apparently it isn't!

It is unambiguous. Neither interpretation says that the bug is newer than nine years vintage.

> Interesting that you and another commenter interpreted "9 year old bug" to mean "bug discovered 9 years ago".

In an unusual moment of reduced cynicism, I too interpreted it as that the bug was newly (or at least recently) discovered whcih does appear to be the case this time.

It could easily have been a bug discovered that long ago that was for some reason marked WONTFIX (the danger not appreciated, that bit of code was at that time due to be deprecated RealSoonNow, ...) or CANTREPRO.

> From the article

Yep. The fault lies with whoever named this submission. It isn't as if they've just used the title of the page linked to, as that differs completely, so a thought process happened and a wording decision was made.

> as that differs completely

There must still be confusion, as when I read the title it means exactly what is said in the GitHub issue.

Reading an oversized int from the network just keeps reading bytes unbounded eh? Do you have to provide an insanely long int or could it be combined with a slow loris?

It's basically slowloris-- send 0xFF every 20 seconds to keep the varint reader blocking waiting for the end of the number.

Aiee, code is hard. Great work to all involved. Glad the good guys found this one first.

Quite an interesting read!

Reply on news.ycombinator.com