martinthomson / ohttp

Rust library for encapsulating HTTP messages in a cryptographic wrapper
Apache License 2.0
22 stars 16 forks source link

consider making it harder to cause the bhttp parser to allocate a massive amount of memory #35

Closed mozkeeler closed 1 year ago

mozkeeler commented 1 year ago

Attempting to decode the bhttp message 03678900e4cb0ab03115fa0b4c2ea0fa783f7a87fa00 causes the implementation to attempt to allocate a massive amount of memory. Perhaps we should take into account the length of the input when allocating in read_vec?

martinthomson commented 1 year ago

The interface deliberately elides the information that might allow it to avoid that massive allocation. However, it doesn't make sense to do that without an asynchronous interface. Perhaps the right answer here is to assume that the input is all available.

To do that, the input to this function could be minimally changed to require BufRead + Seek. Then we can reimplement Seek::stream_len() (until it stabilizes) to determine whether this input is indeed too short. Error::Truncated follows naturally from that.

A streaming API can then use a different approach (where data is both obtained and forwarded in chunks, asynchronously).

Sound reasonable?

mozkeeler commented 1 year ago

Sure - sounds good. I don't think we're in a particular hurry here anyway, as this shouldn't be exposed to entirely untrusted input for a bit, at least in Firefox.