From bbfda83044599ff7a23c8ca53016b65415f1aa37 Mon Sep 17 00:00:00 2001 From: Eli Collins Date: Tue, 1 May 2012 21:25:36 +0000 Subject: [PATCH] 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/CHANGES.txt | 3 ++ .../org/apache/hadoop/io/WritableUtils.java | 36 +++++++++++++++++-- .../token/delegation/DelegationKey.java | 7 +++- .../apache/hadoop/io/TestWritableUtils.java | 25 +++++++++++++ 4 files changed, 68 insertions(+), 3 deletions(-) diff --git a/hadoop-common-project/hadoop-common/CHANGES.txt b/hadoop-common-project/hadoop-common/CHANGES.txt index 189ddd3cc4..d21cfd71dd 100644 --- a/hadoop-common-project/hadoop-common/CHANGES.txt +++ b/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) diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/WritableUtils.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/WritableUtils.java index db2dde781f..a11a86008d 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/WritableUtils.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/WritableUtils.java @@ -326,9 +326,41 @@ public static long readVLong(DataInput stream) throws IOException { * @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; + } + /** * Given the first byte of a vint/vlong, determine the sign * @param value the first byte diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/token/delegation/DelegationKey.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/token/delegation/DelegationKey.java index 7cea67923e..3b5705eb6d 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/token/delegation/DelegationKey.java +++ b/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 DelegationKey(int keyId, long expiryDate, byte[] encodedKey) { 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 void write(DataOutput out) throws IOException { 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 { diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/TestWritableUtils.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/TestWritableUtils.java index 2487fc0612..a1b2c31924 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/TestWritableUtils.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/TestWritableUtils.java @@ -44,6 +44,26 @@ public static void testValue(int val, int vintlen) throws IOException { assertEquals(vintlen, WritableUtils.getVIntSize(val)); 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); @@ -61,5 +81,10 @@ public static void testVInt() throws Exception { 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); } }