Stack overflow when parsing message

[dbrgn]

When parsing certain messages, the process aborts with a stack overflow.

I made a reproducer:

prost-stackoverflow-reproducer$ cargo run
     Running `target/debug/prost-stackoverflow-reproducer`
Hello, world!

thread 'main' has overflowed its stack
fatal runtime error: stack overflow
Aborted (core dumped)

Potentially the data requests allocation of a buffer that's larger than the actually available data? (23 KiB input data is not that large...)

(Found through afl.rs)

[danburkert]

Wow nice, thanks for the report. Getting a stack trace, if possible, would be super great if you are still looking at this (otherwise no worries, it should be easy to repro thanks to your repo). We have some fuzzers in-tree, but they haven't dug this up. That's something to be investigated as well, or maybe we can just upstream you're test into prost.

[danburkert]

I found it, I believe it's this recursive call within skip_field: https://github.com/danburkert/prost/blob/master/src/encoding.rs#L402. It should be straightforward to restructure that as a loop instead of recursion. Interestingly it doesn't manifest in release mode.

[danburkert]

Interestingly it doesn't manifest in release mode.

It likely would with longer input.

[danburkert]

@dbrgn thanks again for the report, and thorough repro. Fix has been released in 0.6.1; more details are in the release notes: https://github.com/danburkert/prost/releases/tag/v0.6.1. Additionally I filed #270 to track better fuzz testing through afl.

[dbrgn]

Great, thank you for the very fast response! 🙂

I will also file a RUSTSEC advisory, since this could be used for DoS if untrusted input is fed to the parser.

[tarcieri]

Note that in absence of stack probes on architectures like ARM, stack overflow is unsound and can result in potential memory corruption (or even RCE), so it's worse than DoS.

Fortunately x86/x86_64 has stack probes.

[dbrgn]

Oh, that's a good point! I wasn't aware of that.

I'll file a RUSTSEC advisory. In that case, should I put it both in "denial-of-service" and "memory-corruption" categories?

[tarcieri]

Yup, sounds good.

[tarcieri]

Assigned RUSTSEC-2020-0002 in rustsec/advisory-db#223

[mzabaluev]

Are 0.5 versions affected? If so, is it possible to backport the fix?

[danburkert]

0.5.x have no stack overflow protections at all, so yes. There will be no backport, as I don't have the resources to maintain backports.