lib0 icon indicating copy to clipboard operation
lib0 copied to clipboard

[decoding] add limits to variable-length decoders

Open reknih opened this issue 8 months ago • 1 comments

This commit allows decoder users to limit how many characters or array elements they want to decode.

Such limits are useful in a server environment because lib0 synchronously decodes messages. An attacker just send a message with a very long string or array and stall the main thread of the server so that no other requests can be served.

The length limits have been added to these public functions:

  • readVarUint8Array
  • readTerminatedUint8Array
  • readVarString
  • peekVarString
  • readTerminatedString
  • The constructor of StringDecoder

Each of these functions accepts a new trailing optional argument maxLen. The change should therefore not be breaking.

I have added some tests, the code is fully covered.

reknih avatar May 08 '25 20:05 reknih

Wouldn't it be better to check the length of the buffer instead?

If you really must check the size of readVar* decoders without reading the content, you could do peekVarUint(decoder) > maxLen instead before reading the content.

Generally, I avoid polymorphism in low-level primitives. This is why this library is so fast. Hence, I don't want to accept the PR as is. Sorry for that. There are probably better ways to solve the problem you are facing.

I would accept a PR that checks that the buffer is not exceeded (which is not always the case in lib0).

dmonad avatar May 09 '25 12:05 dmonad