Bound gRPC frame sizes#220
Conversation
Reject oversized server-streaming gRPC messages as soon as their frame header is available. Also reject compressed stream frames consistently and avoid unchecked frame length arithmetic before slicing buffers.
Read unary client responses incrementally and reject bodies above the configured limit before protobuf decoding. Preallocate only after a valid Content-Length has been checked.
|
I've assigned @valentinewallace as a reviewer! |
|
|
||
| // Applies to complete unary gRPC responses. The server applies the same cap to unary request | ||
| // bodies before protobuf decoding. | ||
| const MAX_GRPC_UNARY_RESPONSE_LEN: usize = 10 * 1024 * 1024; |
There was a problem hiding this comment.
wdyt about having the unit explicit in the const name? _BYTES instead of _LEN?
| Ok(payload) | ||
| } | ||
|
|
||
| fn check_grpc_unary_response_len(len: u64) -> Result<(), LdkServerError> { |
There was a problem hiding this comment.
if you drop the content_size path, you can change this to receive usize and avoid the as castings both on L535 and L542
| content_length as usize | ||
| } else { | ||
| 0 | ||
| }; |
There was a problem hiding this comment.
I think the Some path is essentially useless as ldk-server doesn't set the content length header. It actually could't even if it wanted to. In gRPC the body streams and you can't compute a length.
Also, maybe worth noting that having Vec::with_capacity(0) hardcoded it's kinda bad because it will always have to allocate. Given the context where this is running it seems like not that big of a deal. You could initialize for something different than zero, or maybe use some BytesMut trickery to alloc exactly what you need.
3 participants
Reject oversized server-streaming gRPC messages as soon as their frame
header is available. Also reject compressed stream frames consistently
and avoid unchecked frame length arithmetic before slicing buffers.
Read unary client responses incrementally and reject bodies above the
configured limit before protobuf decoding. Preallocate only after a
valid Content-Length has been checked.
tested with all the graph clis and they still fit within the limit