From 469c1ca34c7b4efa79283f9749db0dc5c84f5d44 Mon Sep 17 00:00:00 2001
From: Norman Maurer <norman_maurer@apple.com>
Date: Tue, 21 Apr 2020 12:04:25 +0200
Subject: [PATCH] Guard against overflow when calling
 CompositeByteBuf.addComponent(...) (#10197)

Motivation:

We need to ensure we not overflow when adding buffers to a CompositeByteBuf

Modifications:

- Correctly validate overflow before adding to the internal storage
- Add testcase

Result:

Fixes https://github.com/netty/netty/issues/10194
---
 .../io/netty/buffer/CompositeByteBuf.java     | 18 +++++++
 .../buffer/AbstractCompositeByteBufTest.java  | 52 +++++++++++++++++++
 2 files changed, 70 insertions(+)

diff --git a/buffer/src/main/java/io/netty/buffer/CompositeByteBuf.java b/buffer/src/main/java/io/netty/buffer/CompositeByteBuf.java
index 37ede17907..41b7dbcba4 100644
--- a/buffer/src/main/java/io/netty/buffer/CompositeByteBuf.java
+++ b/buffer/src/main/java/io/netty/buffer/CompositeByteBuf.java
@@ -279,6 +279,12 @@ public class CompositeByteBuf extends AbstractReferenceCountedByteBuf implements
             Component c = newComponent(ensureAccessible(buffer), 0);
             int readableBytes = c.length();
 
+            // Check if we would overflow.
+            // See https://github.com/netty/netty/issues/10194
+            if (capacity() + readableBytes < 0) {
+                throw new IllegalArgumentException("Can't increase by " + readableBytes);
+            }
+
             addComp(cIndex, c);
             wasAdded = true;
             if (readableBytes > 0 && cIndex < componentCount - 1) {
@@ -359,6 +365,18 @@ public class CompositeByteBuf extends AbstractReferenceCountedByteBuf implements
     private CompositeByteBuf addComponents0(boolean increaseWriterIndex,
             final int cIndex, ByteBuf[] buffers, int arrOffset) {
         final int len = buffers.length, count = len - arrOffset;
+
+        int readableBytes = 0;
+        int capacity = capacity();
+        for (int i = 0; i < buffers.length; i++) {
+            readableBytes += buffers[i].readableBytes();
+
+            // Check if we would overflow.
+            // See https://github.com/netty/netty/issues/10194
+            if (capacity + readableBytes < 0) {
+                throw new IllegalArgumentException("Can't increase by " + readableBytes);
+            }
+        }
         // only set ci after we've shifted so that finally block logic is always correct
         int ci = Integer.MAX_VALUE;
         try {
diff --git a/buffer/src/test/java/io/netty/buffer/AbstractCompositeByteBufTest.java b/buffer/src/test/java/io/netty/buffer/AbstractCompositeByteBufTest.java
index 7d25e57e83..eca7098435 100644
--- a/buffer/src/test/java/io/netty/buffer/AbstractCompositeByteBufTest.java
+++ b/buffer/src/test/java/io/netty/buffer/AbstractCompositeByteBufTest.java
@@ -22,6 +22,7 @@ import org.junit.Test;
 import java.nio.ByteBuffer;
 import java.nio.ByteOrder;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Collections;
 import java.util.ConcurrentModificationException;
 import java.util.Iterator;
@@ -1524,4 +1525,55 @@ public abstract class AbstractCompositeByteBufTest extends AbstractByteBufTest {
         }
         assertTrue(cbuf.release());
     }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void testOverflowWhileAddingComponent() {
+        int capacity = 1024 * 1024; // 1MB
+        ByteBuf buffer = Unpooled.buffer(capacity).writeZero(capacity);
+        CompositeByteBuf compositeByteBuf = compositeBuffer(Integer.MAX_VALUE);
+
+        try {
+            for (int i = 0; i >= 0; i += buffer.readableBytes()) {
+                ByteBuf duplicate = buffer.duplicate();
+                compositeByteBuf.addComponent(duplicate);
+                duplicate.retain();
+            }
+        } finally {
+            compositeByteBuf.release();
+        }
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void testOverflowWhileAddingComponentsViaVarargs() {
+        int capacity = 1024 * 1024; // 1MB
+        ByteBuf buffer = Unpooled.buffer(capacity).writeZero(capacity);
+        CompositeByteBuf compositeByteBuf = compositeBuffer(Integer.MAX_VALUE);
+
+        try {
+            for (int i = 0; i >= 0; i += buffer.readableBytes()) {
+                ByteBuf duplicate = buffer.duplicate();
+                compositeByteBuf.addComponents(duplicate);
+                duplicate.retain();
+            }
+        } finally {
+            compositeByteBuf.release();
+        }
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void testOverflowWhileAddingComponentsViaIterable() {
+        int capacity = 1024 * 1024; // 1MB
+        ByteBuf buffer = Unpooled.buffer(capacity).writeZero(capacity);
+        CompositeByteBuf compositeByteBuf = compositeBuffer(Integer.MAX_VALUE);
+
+        try {
+            for (int i = 0; i >= 0; i += buffer.readableBytes()) {
+                ByteBuf duplicate = buffer.duplicate();
+                compositeByteBuf.addComponents(Collections.singletonList(duplicate));
+                duplicate.retain();
+            }
+        } finally {
+            compositeByteBuf.release();
+        }
+    }
 }