NC-1026 - Validate Frame Length when receiving data (#30)

* NC-1026 Make Framer#frame Throw on Excessively Large messages

Note that this is

* NC-1026 Make Framer#frame Throw on Excessively Large messages

It is impossible to receive an uncompressed message < 16MiB because the length field in the header is 3 bytes. So this test will never pass.

* NC-1026 Make Framer#frame Throw on Excessively Large messages

review changes

* NC-1026 Make Framer#frame Throw on Excessively Large messages

Missed change in exception type.  Added Null uncompressedLength test.

ethereum/p2p/src/main/java/net/consensys/pantheon/ethereum/p2p/rlpx/framing/Compressor.java

@@ -22,4 +22,14 @@ public interface Compressor {
    * @return The original payload.
    */
   byte[] decompress(byte[] compressed) throws CompressionException;
+
+  /**
+   * Return the length when uncompressed
+   *
+   * @param compressed The compressed payload.
+   * @return The length of the payload when uncompressed.
+   * @throws CompressionException Thrown if the size cannot be calculated from the available data;
+   *     expect to find the root cause inside;
+   */
+  int uncompressedLength(byte[] compressed) throws CompressionException;
 }

ethereum/p2p/src/main/java/net/consensys/pantheon/ethereum/p2p/rlpx/framing/Framer.java

@@ -15,6 +15,7 @@ import net.consensys.pantheon.util.bytes.BytesValue;
 
 import java.util.Arrays;
 
+import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Preconditions;
 import io.netty.buffer.ByteBuf;
 import org.bouncycastle.crypto.BlockCipher;
@@ -212,7 +213,6 @@ public class Framer {
    *
    * @param f The buffer containing
    * @param frameSize The expected
-   * @return
    */
   private MessageData processFrame(final ByteBuf f, final int frameSize) {
     final int pad = padding16(frameSize);
@@ -242,10 +242,14 @@ public class Framer {
     final int id = idbv.isZero() || idbv.size() == 0 ? 0 : idbv.get(0);
 
     // Write message data to ByteBuf, decompressing as necessary
-    ByteBuf data;
+    final ByteBuf data;
     if (compressionEnabled) {
       // Decompress data before writing to ByteBuf
       final byte[] compressedMessageData = Arrays.copyOfRange(frameData, 1, frameData.length - pad);
+      // Check message length
+      Preconditions.checkState(
+          compressor.uncompressedLength(compressedMessageData) < LENGTH_MAX_MESSAGE_FRAME,
+          "Message size in excess of maximum length.");
       final byte[] decompressedMessageData = compressor.decompress(compressedMessageData);
       data = NetworkMemoryPool.allocate(decompressedMessageData.length);
       data.writeBytes(decompressedMessageData);
@@ -301,7 +305,8 @@ public class Framer {
     }
   }
 
-  private ByteBuf frameAndReleaseMessage(final MessageData message) {
+  @VisibleForTesting
+  ByteBuf frameAndReleaseMessage(final MessageData message) {
     try {
       final int frameSize = message.getSize() + LENGTH_MESSAGE_ID;
       final int pad = padding16(frameSize);

ethereum/p2p/src/main/java/net/consensys/pantheon/ethereum/p2p/rlpx/framing/SnappyCompressor.java

@@ -1,6 +1,6 @@
 package net.consensys.pantheon.ethereum.p2p.rlpx.framing;
 
-import static com.google.common.base.Preconditions.checkArgument;
+import static com.google.common.base.Preconditions.checkNotNull;
 
 import java.io.IOException;
 
@@ -15,7 +15,7 @@ public class SnappyCompressor implements Compressor {
 
   @Override
   public byte[] compress(final byte[] uncompressed) {
-    checkArgument(uncompressed != null, "input data must not be null");
+    checkNotNull(uncompressed, "input data must not be null");
     try {
       return Snappy.compress(uncompressed);
     } catch (final IOException e) {
@@ -25,11 +25,21 @@ public class SnappyCompressor implements Compressor {
 
   @Override
   public byte[] decompress(final byte[] compressed) {
-    checkArgument(compressed != null, "input data must not be null");
+    checkNotNull(compressed, "input data must not be null");
     try {
       return Snappy.uncompress(compressed);
     } catch (final IOException e) {
       throw new CompressionException("Snappy decompression failed", e);
     }
   }
+
+  @Override
+  public int uncompressedLength(final byte[] compressed) {
+    checkNotNull(compressed, "input data must not be null");
+    try {
+      return Snappy.uncompressedLength(compressed);
+    } catch (final IOException e) {
+      throw new CompressionException("Snappy uncompressedLength failed", e);
+    }
+  }
 }

ethereum/p2p/src/test/java/net/consensys/pantheon/ethereum/p2p/rlpx/framing/FramerTest.java

@@ -22,12 +22,13 @@ import com.fasterxml.jackson.databind.ObjectMapper;
 import io.netty.buffer.ByteBuf;
 import io.netty.buffer.Unpooled;
 import org.junit.Test;
+import org.xerial.snappy.Snappy;
 
 public class FramerTest {
   private static final ObjectMapper MAPPER = new ObjectMapper();
 
   @Test
-  public void shouldThrowExceptionWhenMessageTooLong() {
+  public void shouldThrowExceptionWhenFramingMessageTooLong() {
     final byte[] aes = {
       0xa, 0x2, 0xa, 0x2, 0xa, 0x2, 0xa, 0x2, 0xa, 0x2, 0xa, 0x2, 0xa, 0x2, 0xa, 0x2, 0xa, 0x2, 0xa,
       0x2, 0xa, 0x2, 0xa, 0x2, 0xa, 0x2, 0xa, 0x2, 0xa, 0x2, 0xa, 0x2
@@ -50,6 +51,34 @@ public class FramerTest {
         .withMessageContaining("Message size in excess of maximum length.");
   }
 
+  @Test
+  public void shouldThrowExceptionWhenDeframingCompressedMessageTooLong() throws IOException {
+    // This is a circular test.
+    //
+    // This test frames a too-large message; it then impersonates the sending end
+    // by swapping the ingress and egress MACs and frames the plaintext messages.
+    // The expected result is an exception on deframing not relating to encryption.
+    //
+    final JsonNode td = MAPPER.readTree(FramerTest.class.getResource("/peer1.json"));
+    final HandshakeSecrets secrets = secretsFrom(td, false);
+    final Framer framer = new Framer(secrets);
+    framer.enableCompression();
+
+    final byte[] byteArray = Snappy.compress(new byte[0x1000000]);
+    final ByteBuf buf = wrappedBuffer(byteArray);
+
+    final MessageData ethMessage = new RawMessage(0x00, buf);
+    final ByteBuf framedMessage = framer.frameAndReleaseMessage(ethMessage);
+
+    final HandshakeSecrets deframeSecrets = secretsFrom(td, true);
+    final Framer deframer = new Framer(deframeSecrets);
+    deframer.enableCompression();
+
+    assertThatExceptionOfType(IllegalStateException.class)
+        .isThrownBy(() -> deframer.deframe(framedMessage))
+        .withMessageContaining("Message size in excess of maximum length.");
+  }
+
   @Test
   public void deframeOne() throws IOException {
     // Load test data.

ethereum/p2p/src/test/java/net/consensys/pantheon/ethereum/p2p/rlpx/framing/SnappyCompressorTest.java

@@ -25,18 +25,24 @@ public class SnappyCompressorTest {
     assertThat(snappy.decompress(snappy.compress(data))).isEqualTo(data);
   }
 
-  @Test(expected = IllegalArgumentException.class)
+  @Test(expected = NullPointerException.class)
   public void compressNull() {
     final SnappyCompressor snappy = new SnappyCompressor();
     snappy.compress(null);
   }
 
-  @Test(expected = IllegalArgumentException.class)
+  @Test(expected = NullPointerException.class)
   public void decompressNull() {
     final SnappyCompressor snappy = new SnappyCompressor();
     snappy.decompress(null);
   }
 
+  @Test(expected = NullPointerException.class)
+  public void uncompressedLengthNull() {
+    final SnappyCompressor snappy = new SnappyCompressor();
+    snappy.uncompressedLength(null);
+  }
+
   @Test
   public void roundTripEthereumData() {
     // First data set.