HADOOP-8275. Range check DelegationKey length. Contributed by Colin Patrick McCabe

git-svn-id: https://svn.apache.org/repos/asf/hadoop/common/trunk@1332839 13f79535-47bb-0310-9956-ffa450edef68

hadoop-common-project/hadoop-common/CHANGES.txt

@@ -392,6 +392,9 @@ Release 2.0.0 - UNRELEASED
     HADOOP-8325. Add a ShutdownHookManager to be used by different
     components instead of the JVM shutdownhook (tucu)
 
+    HADOOP-8275. Range check DelegationKey length.
+    (Colin Patrick McCabe via eli)
+
   BREAKDOWN OF HADOOP-7454 SUBTASKS
 
     HADOOP-7455. HA: Introduce HA Service Protocol Interface. (suresh)

hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/WritableUtils.java

@@ -326,7 +326,39 @@ public final class WritableUtils  {
    * @return deserialized integer from stream.
    */
   public static int readVInt(DataInput stream) throws IOException {
-    return (int) readVLong(stream);
+    long n = readVLong(stream);
+    if ((n > Integer.MAX_VALUE) || (n < Integer.MIN_VALUE)) {
+      throw new IOException("value too long to fit in integer");
+    }
+    return (int)n;
+  }
+
+  /**
+   * Reads an integer from the input stream and returns it.
+   *
+   * This function validates that the integer is between [lower, upper],
+   * inclusive.
+   *
+   * @param stream Binary input stream
+   * @throws java.io.IOException
+   * @return deserialized integer from stream
+   */
+  public static int readVIntInRange(DataInput stream, int lower, int upper)
+      throws IOException {
+    long n = readVLong(stream);
+    if (n < lower) {
+      if (lower == 0) {
+        throw new IOException("expected non-negative integer, got " + n);
+      } else {
+        throw new IOException("expected integer greater than or equal to " +
+            lower + ", got " + n);
+      }
+    }
+    if (n > upper) {
+      throw new IOException("expected integer less or equal to " + upper +
+          ", got " + n);
+    }
+    return (int)n;
   }
 
   /**

hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/token/delegation/DelegationKey.java

@@ -41,6 +41,7 @@ public class DelegationKey implements Writable {
   private long expiryDate;
   @Nullable
   private byte[] keyBytes = null;
+  private static final int MAX_KEY_LEN = 1024 * 1024;
 
   /** Default constructore required for Writable */
   public DelegationKey() {
@@ -55,6 +56,10 @@ public class DelegationKey implements Writable {
     this.keyId = keyId;
     this.expiryDate = expiryDate;
     if (encodedKey != null) {
+      if (encodedKey.length > MAX_KEY_LEN) {
+        throw new RuntimeException("can't create " + encodedKey.length +
+            " byte long DelegationKey.");
+      }
       this.keyBytes = encodedKey;
     }
   }
@@ -102,7 +107,7 @@ public class DelegationKey implements Writable {
   public void readFields(DataInput in) throws IOException {
     keyId = WritableUtils.readVInt(in);
     expiryDate = WritableUtils.readVLong(in);
-    int len = WritableUtils.readVInt(in);
+    int len = WritableUtils.readVIntInRange(in, -1, MAX_KEY_LEN);
     if (len == -1) {
       keyBytes = null;
     } else {

hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/TestWritableUtils.java

@@ -45,6 +45,26 @@ public class TestWritableUtils extends TestCase {
     assertEquals(vintlen, WritableUtils.decodeVIntSize(buf.getData()[0]));
   }
   
+  public static void testReadInRange(long val, int lower,
+      int upper, boolean expectSuccess) throws IOException {
+    DataOutputBuffer buf = new DataOutputBuffer();
+    DataInputBuffer inbuf = new DataInputBuffer();
+    WritableUtils.writeVLong(buf, val);
+    try {
+      inbuf.reset(buf.getData(), 0, buf.getLength());
+      long val2 = WritableUtils.readVIntInRange(inbuf, lower, upper);
+      if (!expectSuccess) {
+        fail("expected readVIntInRange to throw an exception");
+      }
+      assertEquals(val, val2);
+    } catch(IOException e) {
+      if (expectSuccess) {
+        LOG.error("unexpected exception:", e);
+        fail("readVIntInRange threw an unexpected exception");
+      }
+    }
+  }
+
   public static void testVInt() throws Exception {
     testValue(12, 1);
     testValue(127, 1);
@@ -61,5 +81,10 @@ public class TestWritableUtils extends TestCase {
     testValue(-65536, 3);
     testValue(65536, 4);
     testValue(-65537, 4);
+    testReadInRange(123, 122, 123, true);
+    testReadInRange(123, 0, 100, false);
+    testReadInRange(0, 0, 100, true);
+    testReadInRange(-1, 0, 100, false);
+    testReadInRange(1099511627776L, 0, Integer.MAX_VALUE, false);
   }
 }