Watch out for DoS when using Rust’s Hyper package | Hacker News Reader

Watch out for DoS when using Rust’s Hyper package

jfrog.com

53 pointssimjue19 comments

I'm confused by the fact that there's no mention of interaction with the Hyper project's authors. I'm fairly certain that Sean & contributors will want to address the underlying issue, if they haven't already done so (clearly they were describing the potential for misusing the Hyper API very directly in the docs!), and pointing that out and clearly stating when and how that has been or will be addressed would shine a much more positive light on everyone, including the security researchers.

I see that there's an 1.0 RC release and the offending API seems to have changed and is probably not amenable to this type of misuse any more. The article authors could have easily added some info about that -- I certainly would have appreciated not having to go looking for that myself.

>I'm confused by the fact that there's no mention of interaction with the Hyper project's authors.

Perhaps the view taken is that this isn't a hyper problem? i.e. hyper is open and unrestricted by design and it is the package users responsibility to not point said unrestricted gun at their foot

Agree though that either way some mention of hyper interaction would be good here

> single Bytes buffer, for example the following unsafe usage

Highlighting “unsafe” in red in an article about a Rust package when talking about something which is not Unsafe is so cursed.

The article's usage of "unsafe" seems to be a bit more general than Rust's specific concept of memory safety, hence the confusion. `to_bytes()` is marked safe in Rust code, and it is memory-safe in Rust's terms: you've just asked for a memory block of arbitrary size, there is no undefined behavior. But real-world safety is a bit more general than that, and given Rust's reliance of npm-style package management with lots of dependencies these issues can be exacerbated in scale.

I don't think Hyper itself was the problem though, frameworks like Axum are where things should be fixed. Hyper is intended to be used as a low-level library, so it should allocate exactly what size the programmer requested and the responsibility of length checking should be the system programmer's burden.

That is quite common in some Rust communities that go beyond what unsafe is supposed to do, and see everything that might be wrong usage as unsafe.

Ideally they want to be using formal methods, dependent typing, but instead bring that to Rust and put unsafe even on code that doesn't do anything related to low level memory handling or concurrency.

Maybe there shouldn't be a

    to_bytes(body) -> Bytes

function at all ?

Only a

    to_bytes(body: B, max_size: Option<usize>)

This way if someone REALLY wants the behavior that potentially results in a crash, they still have access to it, but have to be really explicit about it.

Rouille, another Rust HTTP server, can also trivially be DOS'd by sending a Content-Length that doesn't match the actual content length.

But HTTP implementations like these are not really meant to directly face the internet. They usually sit behind reverse proxies/API gateways/CDNs.

Good to see public sharing not only of such a problem, but also how to fix it in your own code.

I am a bit disconcerted that something that apparently is warned against in the docs, is done across several "big" packages that use Hyper. Maybe with a more appropriate name exposed by the library, for example `to_bytes_unchecked`, such "bad" uses would be less wide-spread.

Seemingly related PR https://github.com/hyperium/hyper/pull/3046

https://github.com/tokio-rs/axum/pull/1346

Also for ref :)

I assume most production environments will run a reverse proxy like nginx which have sensible defaults. Good find nonetheless. Should be patched by Hyper.

Will falliable allocations help eliminate these kinds of DoS vulnerabilities?

AFAIK there's a proposal: https://rust-lang.github.io/rfcs/2116-alloc-me-maybe.html

It would be nice if Bytes used it: https://github.com/tokio-rs/bytes/pull/521

but that's not entirely sufficient. Even ignoring overcommit shenanigans of Linux, an ever-growing request can cause a lot of strain on the process or the OS before it finally makes allocations fail.

Another relatively safe strategy : Don't buffer http data on server. Read a limited sized header and then stream the body to its request handler.

Right: like, the entire concept of to_bytes is in a very real sense a dangerous anti-pattern that only feels necessary because so many of our other APIs are so difficult to use on streams.

That's not necessarily better as it's sensitive to slowloris attacks, and the handler might not work with streams either, or could just read it all in memory anyway, so you're just moving the problem around.

Hell, a streaming consumer which doesn't require and check the content-length might even spin forever on CPU if a malicious client keeps sending data.

Recently on HN: nothing can ever happen in rust it's impossible to write bad code

What's interesting to me is we've known about these class of API misdesign issues since gets(3)... sometimes convenient and ergonomic is really not the right way to do it.

But the class here is DoS-able, not buffer overrun. The “fixed” version of gets takes a max string length. This DoS-able function already does that. The concern here is different in that the library’s design encourages using untrusted input (Content-Length) to set the buffer size.

Reply on news.ycombinator.com