Allow a limit to be set on the decompressed buffer size for ZlibDecoders #9924
Conversation
Motivation: It is impossible to know in advance how much memory will be needed to decompress a stream of bytes that was compressed using the DEFLATE algorithm. In theory, up to 1032 times the compressed size could be needed. For untrusted input, an attacker could exploit this to exhaust the memory pool. Modifications: ZlibDecoder and its subclasses now support an optional limit on the size of the decompressed buffer. By default, if the limit is reached, decompression stops and a DecompressionException is thrown. Behavior upon reaching the limit is modifiable by subclasses in case they desire something else. Result: The decompressed buffer can now be limited to a configurable size, thus mitigating the possibility of memory pool exhaustion.
|
Can one of the admins verify this patch? |
|
@netty-bot test this please |
|
Failed due to Checkstyle violation. Should be fixed now. |
|
@netty-bot test this please |
|
@rdicroce did you sign our ICLA ? |
|
Yes, although it's been 4.5 years. My last contribution was #4338. Does the ICLA expire after a certain amount of time? |
|
@rdicroce ah found it... :) |
codec/src/main/java/io/netty/handler/codec/compression/JZlibDecoder.java
Show resolved
Hide resolved
codec/src/main/java/io/netty/handler/codec/compression/JZlibDecoder.java
Show resolved
Hide resolved
| /** | ||
| * Maximum allowed size of the decompression buffer. | ||
| */ | ||
| protected int maxAllocation; |
There was a problem hiding this comment.
can this be private/final?
There was a problem hiding this comment.
I'll make it final but leave it as protected in case a subclass wants to read it for any reason.
codec/src/main/java/io/netty/handler/codec/compression/ZlibDecoder.java
Outdated
Show resolved
Hide resolved
codec/src/main/java/io/netty/handler/codec/compression/JZlibDecoder.java
Show resolved
Hide resolved
codec/src/main/java/io/netty/handler/codec/compression/ZlibDecoder.java
Outdated
Show resolved
Hide resolved
codec/src/main/java/io/netty/handler/codec/compression/ZlibDecoder.java
Outdated
Show resolved
Hide resolved
codec/src/main/java/io/netty/handler/codec/compression/ZlibDecoder.java
Outdated
Show resolved
Hide resolved
|
Pushed another commit that addresses reviewers' comments. |
|
@njhill can you have a look again ? |
codec/src/test/java/io/netty/handler/codec/compression/ZlibTest.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
LGTM apart from my comment #9924 (comment) re adding a clarifying comment to the logic.
|
@netty test this please |
|
@rdicroce please fix check style: |
|
@normanmaurer Done. |
|
@netty-bot test this please |
|
@normanmaurer Build failure is due to constructors on ZlibDecoder being changed from public to protected. Do you want me to undo that? |
|
@rdicroce yes please |
|
@netty-bot test this please |
|
@rdicroce seems to cause some unit tests to fail.. can you check ? |
|
Somehow none of us noticed that testMaxAllocation() was broken - it was using uncompressed data. I've fixed that along with the test failures. |
|
@netty-bot test this please |
|
Test failure looks unrelated. |
|
@netty-bot test this please |
|
@rdicroce thanks a lot! |
…ers (#9924) Motivation: It is impossible to know in advance how much memory will be needed to decompress a stream of bytes that was compressed using the DEFLATE algorithm. In theory, up to 1032 times the compressed size could be needed. For untrusted input, an attacker could exploit this to exhaust the memory pool. Modifications: ZlibDecoder and its subclasses now support an optional limit on the size of the decompressed buffer. By default, if the limit is reached, decompression stops and a DecompressionException is thrown. Behavior upon reaching the limit is modifiable by subclasses in case they desire something else. Result: The decompressed buffer can now be limited to a configurable size, thus mitigating the possibility of memory pool exhaustion.
…ers (netty#9924) Motivation: It is impossible to know in advance how much memory will be needed to decompress a stream of bytes that was compressed using the DEFLATE algorithm. In theory, up to 1032 times the compressed size could be needed. For untrusted input, an attacker could exploit this to exhaust the memory pool. Modifications: ZlibDecoder and its subclasses now support an optional limit on the size of the decompressed buffer. By default, if the limit is reached, decompression stops and a DecompressionException is thrown. Behavior upon reaching the limit is modifiable by subclasses in case they desire something else. Result: The decompressed buffer can now be limited to a configurable size, thus mitigating the possibility of memory pool exhaustion.
Motivation:
It is impossible to know in advance how much memory will be needed to
decompress a stream of bytes that was compressed using the DEFLATE
algorithm. In theory, up to 1032 times the compressed size could be
needed. For untrusted input, an attacker could exploit this to exhaust
the memory pool.
Modifications:
ZlibDecoder and its subclasses now support an optional limit on the size
of the decompressed buffer. By default, if the limit is reached,
decompression stops and a DecompressionException is thrown. Behavior
upon reaching the limit is modifiable by subclasses in case they desire
something else.
Result:
The decompressed buffer can now be limited to a configurable size, thus
mitigating the possibility of memory pool exhaustion.
This fixes #6168 for JZlibDecoder and JdkZlibDecoder.