
Nine-year-old bug in the Go standard library enables DoS - latchkey
https://github.com/ethereum/public-attacknets/issues/12
======
jerf
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](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.

~~~
baby
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/](https://go-
review.googlesource.com/c/go/+/247120/)

~~~
jeffbee
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).

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

~~~
zeeboo
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.

~~~
baby
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.

~~~
morelisp
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.

------
Kobajachi34222
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!

~~~
vlovich123
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.

~~~
remexre
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.)

------
bawolff
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

~~~
antoncohen
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.

~~~
DougBTX
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.

~~~
dspillett
_> 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.

~~~
DougBTX
> 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.

------
donatj
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?

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

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

------
steve1820
Quite an interesting read!

