[I] [improve][broker] Bound decompressed message size before allocation; release ByteBuf on codec exceptions [pulsar]

["zhiyp (via GitHub)" <gi...@apache.org>]

zhiyp opened a new issue, #26442:
URL: https://github.com/apache/pulsar/issues/26442

   ### Search before reporting
   
   - [x] I searched in the [issues](https://github.com/apache/pulsar/issues) and found nothing similar.
   
   
   ### Motivation
   
   When a message is compressed, the decompressed size is taken directly from the uncompressedSize field of MessageMetadata and used to allocate a ByteBuf before decompression. This field is client-controlled, the broker's produce path does not validate it before persisting the message, and most decompression call-sites do not validate it before allocating. A single malformed message can therefore trigger an allocation of up to ~2 GiB (Integer.MAX_VALUE) on any component that later decompresses the entry — even though the compressed payload itself is tiny and passes the frame-size limit.
   
   This is filed as a robustness / defense-in-depth improvement. (Per the documented security model, denial of service by a perimeter-trusted client is outside Pulsar's threat model, so this is not a security report.)
   
   ### Solution
   
    1. Bound uncompressedSize at the produce entry point (preferred — prevents the malformed entry from being persisted): in ServerCnx.handleSend / PersistentTopic.publishMessage, parse the metadata once and reject the send when uncompressedSize > maxMessageSize.
   
    2. Defense-in-depth at every codec.decode(..., getUncompressedSize()) call-site: in MessageParser.uncompressPayloadIfNeeded (add an uncompressedSize > maxMessageSize branch alongside the existing payloadSize check at :160), PersistentTopicsBase:3350, PulsarTopicCompactionService:150, AbstractTwoPhaseCompactor:479, RawBatchConverter:69/150. This also covers entries that bypass the produce path (cross-cluster replication from older brokers, direct BookKeeper tampering).
    
    3. Release ByteBuf on codec exceptions: wrap the allocation + (de)compression in try/finally, releasing the buffer on failure, in CompressionCodecLZ4/Zstd/ZLib/Snappy.encode/decode.
   
   ### Alternatives
   
   _No response_
   
   ### Anything else?
   
   _No response_
   
   ### Are you willing to submit a PR?
   
   - [ ] I'm willing to submit a PR!


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [I] [improve][broker] Bound decompressed message size before allocation; release ByteBuf on codec exceptions [pulsar]

["lhotari (via GitHub)" <gi...@apache.org>]

lhotari commented on issue #26442:
URL: https://github.com/apache/pulsar/issues/26442#issuecomment-6218753160

   perhaps we should document it in Pulsar's threat model that the protocol doesn't cover DoS attacks. Exposing an endpoint to potentially malicious users isn't covered. Therefore additional security measures should be applied. One of the preparations is PIP-478 which decouples the mTLS from authentication. Connecting to an endpoint could require a valid client certificate which could be a way to limit access to an endpoint that could potentially be accessed by malicious actors.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [I] [improve][broker] Bound decompressed message size before allocation; release ByteBuf on codec exceptions [pulsar]

["lhotari (via GitHub)" <gi...@apache.org>]

lhotari commented on issue #26442:
URL: https://github.com/apache/pulsar/issues/26442#issuecomment-5494193601

   Thanks for reporting. However, for future cases, please follow the instructions shared in https://github.com/apache/pulsar/issues/26443#issuecomment-5494172768.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [I] [improve][broker] Bound decompressed message size before allocation; release ByteBuf on codec exceptions [pulsar]

["Asthenia0412 (via GitHub)" <gi...@apache.org>]

Asthenia0412 commented on issue #26442:
URL: https://github.com/apache/pulsar/issues/26442#issuecomment-6214897079

   Submitted PR #26938 to address this. Covers Layer 2 (bound uncompressedSize before ByteBuf allocation at all decompression call sites) and Layer 3 (try/finally release ByteBuf on codec exceptions for all 4 compression codecs). Review welcome!


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org