-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Add decompression size limit to prevent decompression bomb DoS #3625
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 1 commit
50a6bfc
13edbf8
406bf50
fd85f2e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,11 +20,12 @@ | |
| import java.io.IOException; | ||
| import java.io.OutputStream; | ||
| import java.nio.ByteBuffer; | ||
| import java.util.zip.DataFormatException; | ||
| import java.util.zip.Deflater; | ||
| import java.util.zip.DeflaterOutputStream; | ||
| import java.util.zip.Inflater; | ||
| import java.util.zip.InflaterOutputStream; | ||
|
|
||
| import org.apache.avro.AvroRuntimeException; | ||
| import org.apache.avro.util.NonCopyingByteArrayOutputStream; | ||
|
|
||
| /** | ||
|
|
@@ -38,6 +39,20 @@ | |
| public class DeflateCodec extends Codec { | ||
|
|
||
| private static final int DEFAULT_BUFFER_SIZE = 8192; | ||
| private static final String MAX_DECOMPRESS_LENGTH_PROPERTY = "org.apache.avro.limits.decompress.maxLength"; | ||
| private static final long DEFAULT_MAX_DECOMPRESS_LENGTH = 200L * 1024 * 1024; // 200MB default limit | ||
|
|
||
| private static long getMaxDecompressLength() { | ||
| String prop = System.getProperty(MAX_DECOMPRESS_LENGTH_PROPERTY); | ||
| if (prop != null) { | ||
| try { | ||
| return Long.parseLong(prop); | ||
| } catch (NumberFormatException e) { | ||
| // Use default | ||
|
||
| } | ||
| } | ||
| return DEFAULT_MAX_DECOMPRESS_LENGTH; | ||
| } | ||
|
|
||
| static class Option extends CodecFactory { | ||
| private final int compressionLevel; | ||
|
|
@@ -78,10 +93,32 @@ public ByteBuffer compress(ByteBuffer data) throws IOException { | |
|
|
||
| @Override | ||
| public ByteBuffer decompress(ByteBuffer data) throws IOException { | ||
| long maxLength = getMaxDecompressLength(); | ||
|
||
| NonCopyingByteArrayOutputStream baos = new NonCopyingByteArrayOutputStream(DEFAULT_BUFFER_SIZE); | ||
| try (OutputStream outputStream = new InflaterOutputStream(baos, getInflater())) { | ||
| outputStream.write(data.array(), computeOffset(data), data.remaining()); | ||
| byte[] buffer = new byte[DEFAULT_BUFFER_SIZE]; | ||
| long totalBytes = 0; | ||
|
|
||
| Inflater inflater = getInflater(); | ||
| inflater.setInput(data.array(), computeOffset(data), data.remaining()); | ||
|
|
||
| try { | ||
| while (!inflater.finished()) { | ||
| int len = inflater.inflate(buffer); | ||
| if (len == 0 && inflater.needsInput()) { | ||
| break; | ||
| } | ||
| totalBytes += len; | ||
| if (totalBytes > maxLength) { | ||
| throw new AvroRuntimeException( | ||
| "Decompressed size " + totalBytes + " exceeds maximum allowed size " + maxLength | ||
OwenSanzas marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| + ". This can be configured by setting the system property " + MAX_DECOMPRESS_LENGTH_PROPERTY); | ||
OwenSanzas marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| } | ||
| baos.write(buffer, 0, len); | ||
| } | ||
| } catch (DataFormatException e) { | ||
| throw new IOException("Invalid deflate data", e); | ||
| } | ||
|
|
||
| return baos.asByteBuffer(); | ||
| } | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will also accept a negative and 0 as values which are not very sensible.
Probably these should be reported earlier here ?!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done. Added validation for <= 0 values with warning log.