From 583a59abb1c1b2cd99aeac5cbbd2e58c4b087880 Mon Sep 17 00:00:00 2001
From: Scott Mitchell <scott_mitchell@apple.com>
Date: Tue, 3 Jan 2017 09:51:00 -0800
Subject: [PATCH] ByteBufUtil.compare int underflow

Motivation:
ByteBufUtil.compare uses long arithmetic but doesn't check for underflow on when converting from long to int to satisfy the Comparable interface. This will result in incorrect comparisons and violate the Comparable interface contract.

Modifications:
- ByteBufUtil.compare should protect against int underflow

Result:
Fixes https://github.com/netty/netty/issues/6169
---
 .../java/io/netty/buffer/ByteBufUtil.java     |  2 +-
 .../io/netty/buffer/AbstractByteBufTest.java  | 27 ++++++++++++++++---
 2 files changed, 25 insertions(+), 4 deletions(-)

diff --git a/buffer/src/main/java/io/netty/buffer/ByteBufUtil.java b/buffer/src/main/java/io/netty/buffer/ByteBufUtil.java
index 79403c61be..1ddcab5ab0 100644
--- a/buffer/src/main/java/io/netty/buffer/ByteBufUtil.java
+++ b/buffer/src/main/java/io/netty/buffer/ByteBufUtil.java
@@ -246,7 +246,7 @@ public final class ByteBufUtil {
             }
             if (res != 0) {
                 // Ensure we not overflow when cast
-                return (int) Math.min(Integer.MAX_VALUE, res);
+                return (int) Math.min(Integer.MAX_VALUE, Math.max(Integer.MIN_VALUE, res));
             }
             aIndex += uintCountIncrement;
             bIndex += uintCountIncrement;
diff --git a/buffer/src/test/java/io/netty/buffer/AbstractByteBufTest.java b/buffer/src/test/java/io/netty/buffer/AbstractByteBufTest.java
index 35613786b1..720894df29 100644
--- a/buffer/src/test/java/io/netty/buffer/AbstractByteBufTest.java
+++ b/buffer/src/test/java/io/netty/buffer/AbstractByteBufTest.java
@@ -38,6 +38,7 @@ import java.nio.channels.GatheringByteChannel;
 import java.nio.channels.ScatteringByteChannel;
 import java.nio.channels.WritableByteChannel;
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.HashSet;
 import java.util.Random;
 import java.util.Set;
@@ -62,6 +63,8 @@ import static org.junit.Assert.assertSame;
 import static org.junit.Assert.assertThat;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
+import static org.junit.Assume.assumeFalse;
+import static org.junit.Assume.assumeTrue;
 
 /**
  * An abstract test class for channel buffers
@@ -103,6 +106,24 @@ public abstract class AbstractByteBufTest {
         }
     }
 
+    @Test
+    public void comparableInterfaceNotViolated() {
+        assumeFalse(buffer.isReadOnly());
+        buffer.writerIndex(buffer.readerIndex());
+        assumeTrue(buffer.writableBytes() >= 4);
+
+        buffer.writeLong(0);
+        ByteBuf buffer2 = newBuffer(CAPACITY);
+        assumeFalse(buffer2.isReadOnly());
+        buffer2.writerIndex(buffer2.readerIndex());
+        // Write an unsigned integer that will cause buffer.getUnsignedInt() - buffer2.getUnsignedInt() to underflow the
+        // int type and wrap around on the negative side.
+        buffer2.writeLong(0xF0000000L);
+        assertTrue(buffer.compareTo(buffer2) < 0);
+        assertTrue(buffer2.compareTo(buffer) > 0);
+        buffer2.release();
+    }
+
     @Test
     public void initialState() {
         assertEquals(CAPACITY, buffer.capacity());
@@ -1909,7 +1930,7 @@ public abstract class AbstractByteBufTest {
 
     @Test
     public void testNioBuffer1() {
-        Assume.assumeTrue(buffer.nioBufferCount() == 1);
+        assumeTrue(buffer.nioBufferCount() == 1);
 
         byte[] value = new byte[buffer.capacity()];
         random.nextBytes(value);
@@ -1921,7 +1942,7 @@ public abstract class AbstractByteBufTest {
 
     @Test
     public void testToByteBuffer2() {
-        Assume.assumeTrue(buffer.nioBufferCount() == 1);
+        assumeTrue(buffer.nioBufferCount() == 1);
 
         byte[] value = new byte[buffer.capacity()];
         random.nextBytes(value);
@@ -1947,7 +1968,7 @@ public abstract class AbstractByteBufTest {
 
     @Test
     public void testToByteBuffer3() {
-        Assume.assumeTrue(buffer.nioBufferCount() == 1);
+        assumeTrue(buffer.nioBufferCount() == 1);
 
         assertEquals(buffer.order(), buffer.nioBuffer().order());
     }