Duplicate data in stream frames can lead to unbounded resource consumption about quinn

[Ralith]

This also partially defeats the protections added in #330.

[fairingrey]

Not sure if this is the fastest for your uses, but I've tried tackling this issue through https://github.com/fairingrey/byte_sequencer.

It makes one assumption:

• The stream isn't corrupted -- That is, duplicate bytes received in a stream frame will always remain the same

It also won't pop redundant chunks from the binary heap in the case that a larger surrounding chunk is received (this is another worst case scenario). If a chunk of bytes in the range [2,3) is received first, and then a chunk of bytes from [1,4) or [0,5) is received afterwards, then the sequencer will continue to write both to the buffer despite the redundant information in the first chunk (it will write the second first, as it has an offset closer to 0).

Would love some improvement and review if possible, since I don't often write high-performance components...

[Ralith]

The stream isn't corrupted -- That is, duplicate bytes received in a stream frame will always remain the same

This assumption is endorsed by the QUIC draft, so as long as resource consumption remains sane and behavior remains well-defined, relying on it is just fine.

[Ralith]

Per discussion on the rust community discord, as of this writing the above code can be induced by an attacker to consume O(n²) space, which is better than unbounded but still not ideal.

[djc]

I'm guessing we should maybe introduce a different mode here, where we switch to a different approach when we detect that the peer is doing something that might result in bad performance. I suppose we can still get rid of the duplicate data without a large performance penalty, but then we potentially still have the problem fragmented memory slices keeping many largish allocations alive.

[Ralith]

I think an adequate solution to that would be copying stream data into fresh allocations whenever it's less than, say, half the size of the UDP buffer it was read into.

[Ralith]

I suspect both of these were fixed by #598, but we should verify carefully.

[Ralith]

I've reviewed the current behavior. The first case was indeed fixed by #598, but the latter case (roughly) remains. Closing in favor of #735 which describes the remaining issue in greater detail.