You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@commons.apache.org by gg...@apache.org on 2019/05/20 15:39:22 UTC
[commons-codec] branch master updated: CODEC-134: Update
commons-codec to reject decoding any impossible string encoding for Base32
and Base64. (#19)
This is an automated email from the ASF dual-hosted git repository.
ggregory pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/commons-codec.git
The following commit(s) were added to refs/heads/master by this push:
new 48b6157 CODEC-134: Update commons-codec to reject decoding any impossible string encoding for Base32 and Base64. (#19)
48b6157 is described below
commit 48b615756d1d770091ea3322eefc08011ee8b113
Author: tmousaw-ptc <50...@users.noreply.github.com>
AuthorDate: Mon May 20 11:39:18 2019 -0400
CODEC-134: Update commons-codec to reject decoding any impossible string encoding for Base32 and Base64. (#19)
[CODEC-134] Squash and merge.
* CODEC-134: Update to reject decoding strings that are not possible in the given encoding for Base32 and Base64.
* CODEC-134: Fix tabs instead of spaces.
* CODEC-134: Update such that the right shift is not indirectly accomplished via a method call.
* CODEC-134: Fix spaces versus tabs (again).
* CODEC-134: Add test to cover missed exception case in BCodec.java.
* CODEC-134: Update changes.xml to describe change.
---
src/changes/changes.xml | 1 +
.../org/apache/commons/codec/binary/Base32.java | 25 ++++++++++-
.../org/apache/commons/codec/binary/Base64.java | 21 +++++++++-
.../java/org/apache/commons/codec/net/BCodec.java | 2 +-
.../apache/commons/codec/binary/Base32Test.java | 49 ++++++++++++++++++++++
.../apache/commons/codec/binary/Base64Test.java | 19 +++++++++
.../commons/codec/binary/Base64TestData.java | 2 +-
.../org/apache/commons/codec/net/BCodecTest.java | 19 +++++++++
8 files changed, 134 insertions(+), 4 deletions(-)
diff --git a/src/changes/changes.xml b/src/changes/changes.xml
index 18a6cf4..357bae9 100644
--- a/src/changes/changes.xml
+++ b/src/changes/changes.xml
@@ -45,6 +45,7 @@ The <action> type attribute can be add,update,fix,remove.
<release version="1.13" date="YYYY-MM-DD" description="TBD">
<action issue="CODEC-257" dev="ggregory" type="update">Update from Java 7 to Java 8</action>
+ <action issue="CODEC-134" dev="tmousaw-ptc" type="fix">Reject any decode request for a value that is impossible to encode to for Base32/Base64 rather than blindly decoding.</action>
</release>
<release version="1.12" date="2019-02-04" description="Feature and fix release.">
diff --git a/src/main/java/org/apache/commons/codec/binary/Base32.java b/src/main/java/org/apache/commons/codec/binary/Base32.java
index 22c1d65..3301ed4 100644
--- a/src/main/java/org/apache/commons/codec/binary/Base32.java
+++ b/src/main/java/org/apache/commons/codec/binary/Base32.java
@@ -332,7 +332,7 @@ public class Base32 extends BaseNCodec {
* @param inPos
* Position to start reading data from.
* @param inAvail
- * Amount of bytes available from input for encoding.
+ * Amount of bytes available from input for decoding.
* @param context the context to be used
*
* Output is written to {@link Context#buffer} as 8-bit octets, using {@link Context#pos} as the buffer position
@@ -381,29 +381,35 @@ public class Base32 extends BaseNCodec {
// we ignore partial bytes, i.e. only multiples of 8 count
switch (context.modulus) {
case 2 : // 10 bits, drop 2 and output one byte
+ validateCharacter(2, context);
buffer[context.pos++] = (byte) ((context.lbitWorkArea >> 2) & MASK_8BITS);
break;
case 3 : // 15 bits, drop 7 and output 1 byte
+ validateCharacter(7, context);
buffer[context.pos++] = (byte) ((context.lbitWorkArea >> 7) & MASK_8BITS);
break;
case 4 : // 20 bits = 2*8 + 4
+ validateCharacter(4, context);
context.lbitWorkArea = context.lbitWorkArea >> 4; // drop 4 bits
buffer[context.pos++] = (byte) ((context.lbitWorkArea >> 8) & MASK_8BITS);
buffer[context.pos++] = (byte) ((context.lbitWorkArea) & MASK_8BITS);
break;
case 5 : // 25bits = 3*8 + 1
+ validateCharacter(1, context);
context.lbitWorkArea = context.lbitWorkArea >> 1;
buffer[context.pos++] = (byte) ((context.lbitWorkArea >> 16) & MASK_8BITS);
buffer[context.pos++] = (byte) ((context.lbitWorkArea >> 8) & MASK_8BITS);
buffer[context.pos++] = (byte) ((context.lbitWorkArea) & MASK_8BITS);
break;
case 6 : // 30bits = 3*8 + 6
+ validateCharacter(6, context);
context.lbitWorkArea = context.lbitWorkArea >> 6;
buffer[context.pos++] = (byte) ((context.lbitWorkArea >> 16) & MASK_8BITS);
buffer[context.pos++] = (byte) ((context.lbitWorkArea >> 8) & MASK_8BITS);
buffer[context.pos++] = (byte) ((context.lbitWorkArea) & MASK_8BITS);
break;
case 7 : // 35 = 4*8 +3
+ validateCharacter(3, context);
context.lbitWorkArea = context.lbitWorkArea >> 3;
buffer[context.pos++] = (byte) ((context.lbitWorkArea >> 24) & MASK_8BITS);
buffer[context.pos++] = (byte) ((context.lbitWorkArea >> 16) & MASK_8BITS);
@@ -540,4 +546,21 @@ public class Base32 extends BaseNCodec {
public boolean isInAlphabet(final byte octet) {
return octet >= 0 && octet < decodeTable.length && decodeTable[octet] != -1;
}
+
+ /**
+ * <p>
+ * Validates whether the character is possible in the context of the set of possible base 32 values.
+ * </p>
+ *
+ * @param numBits number of least significant bits to check
+ * @param context the context to be used
+ *
+ * @throws IllegalArgumentException if the bits being checked contain any non-zero value
+ */
+ private void validateCharacter(int numBits, Context context) {
+ if ((context.lbitWorkArea & numBits) != 0) {
+ throw new IllegalArgumentException(
+ "Last encoded character (before the paddings if any) is a valid base 32 alphabet but not a possible value");
+ }
+ }
}
diff --git a/src/main/java/org/apache/commons/codec/binary/Base64.java b/src/main/java/org/apache/commons/codec/binary/Base64.java
index 3641c5c..06e2db7 100644
--- a/src/main/java/org/apache/commons/codec/binary/Base64.java
+++ b/src/main/java/org/apache/commons/codec/binary/Base64.java
@@ -421,7 +421,7 @@ public class Base64 extends BaseNCodec {
* @param inPos
* Position to start reading data from.
* @param inAvail
- * Amount of bytes available from input for encoding.
+ * Amount of bytes available from input for decoding.
* @param context
* the context to be used
*/
@@ -469,10 +469,12 @@ public class Base64 extends BaseNCodec {
// TODO not currently tested; perhaps it is impossible?
break;
case 2 : // 12 bits = 8 + 4
+ validateCharacter(4, context);
context.ibitWorkArea = context.ibitWorkArea >> 4; // dump the extra 4 bits
buffer[context.pos++] = (byte) ((context.ibitWorkArea) & MASK_8BITS);
break;
case 3 : // 18 bits = 8 + 8 + 2
+ validateCharacter(2, context);
context.ibitWorkArea = context.ibitWorkArea >> 2; // dump 2 bits
buffer[context.pos++] = (byte) ((context.ibitWorkArea >> 8) & MASK_8BITS);
buffer[context.pos++] = (byte) ((context.ibitWorkArea) & MASK_8BITS);
@@ -781,4 +783,21 @@ public class Base64 extends BaseNCodec {
return octet >= 0 && octet < decodeTable.length && decodeTable[octet] != -1;
}
+ /**
+ * <p>
+ * Validates whether the character is possible in the context of the set of possible base 64 values.
+ * </p>
+ *
+ * @param numBits number of least significant bits to check
+ * @param context the context to be used
+ *
+ * @throws IllegalArgumentException if the bits being checked contain any non-zero value
+ */
+ private long validateCharacter(int numBitsToDrop, Context context) {
+ if ((context.ibitWorkArea & numBitsToDrop) != 0) {
+ throw new IllegalArgumentException(
+ "Last encoded character (before the paddings if any) is a valid base 64 alphabet but not a possible value");
+ }
+ return context.ibitWorkArea >> numBitsToDrop;
+ }
}
diff --git a/src/main/java/org/apache/commons/codec/net/BCodec.java b/src/main/java/org/apache/commons/codec/net/BCodec.java
index 5940f8f..cdaedee 100644
--- a/src/main/java/org/apache/commons/codec/net/BCodec.java
+++ b/src/main/java/org/apache/commons/codec/net/BCodec.java
@@ -178,7 +178,7 @@ public class BCodec extends RFC1522Codec implements StringEncoder, StringDecoder
}
try {
return this.decodeText(value);
- } catch (final UnsupportedEncodingException e) {
+ } catch (final UnsupportedEncodingException | IllegalArgumentException e) {
throw new DecoderException(e.getMessage(), e);
}
}
diff --git a/src/test/java/org/apache/commons/codec/binary/Base32Test.java b/src/test/java/org/apache/commons/codec/binary/Base32Test.java
index 734b8bb..547c241 100644
--- a/src/test/java/org/apache/commons/codec/binary/Base32Test.java
+++ b/src/test/java/org/apache/commons/codec/binary/Base32Test.java
@@ -46,6 +46,30 @@ public class Base32Test {
{"foobar" ,"MZXW6YTBOI======"},
};
+ private static final String[] BASE32_IMPOSSIBLE_CASES = {
+ "MC======",
+ "MZXE====",
+ "MZXWB===",
+ "MZXW6YB=",
+ "MZXW6YTBOC======"
+ };
+
+ private static final String[] BASE32_IMPOSSIBLE_CASES_CHUNKED = {
+ "M2======\r\n",
+ "MZX0====\r\n",
+ "MZXW0===\r\n",
+ "MZXW6Y2=\r\n",
+ "MZXW6YTBO2======\r\n"
+ };
+
+ private static final String[] BASE32HEX_IMPOSSIBLE_CASES = {
+ "C2======",
+ "CPN4====",
+ "CPNM1===",
+ "CPNMUO1=",
+ "CPNMUOJ1E2======"
+ };
+
private static final Object[][] BASE32_BINARY_TEST_CASES;
// { null, "O0o0O0o0" }
@@ -248,4 +272,29 @@ public class Base32Test {
}
}
+ @Test
+ public void testBase32ImpossibleSamples() {
+ testImpossibleCases(new Base32(), BASE32_IMPOSSIBLE_CASES);
+ }
+
+ @Test
+ public void testBase32ImpossibleChunked() {
+ testImpossibleCases(new Base32(20), BASE32_IMPOSSIBLE_CASES_CHUNKED);
+ }
+
+ @Test
+ public void testBase32HexImpossibleSamples() {
+ testImpossibleCases(new Base32(true), BASE32HEX_IMPOSSIBLE_CASES);
+ }
+
+ private void testImpossibleCases(Base32 codec, String[] impossible_cases) {
+ for (String impossible : impossible_cases) {
+ try {
+ codec.decode(impossible);
+ fail();
+ } catch (IllegalArgumentException ex) {
+ // expected
+ }
+ }
+ }
}
diff --git a/src/test/java/org/apache/commons/codec/binary/Base64Test.java b/src/test/java/org/apache/commons/codec/binary/Base64Test.java
index 4206cab..5bec44e 100644
--- a/src/test/java/org/apache/commons/codec/binary/Base64Test.java
+++ b/src/test/java/org/apache/commons/codec/binary/Base64Test.java
@@ -44,6 +44,13 @@ public class Base64Test {
private static final Charset CHARSET_UTF8 = Charsets.UTF_8;
+ private static final String[] BASE64_IMPOSSIBLE_CASES = {
+ "ZE==",
+ "ZmC=",
+ "Zm9vYE==",
+ "Zm9vYmC=",
+ };
+
private final Random random = new Random();
/**
@@ -1297,4 +1304,16 @@ public class Base64Test {
assertEquals("testDEFAULT_BUFFER_SIZE", strOriginal, strDecoded);
}
+ @Test
+ public void testBase64ImpossibleSamples() {
+ Base64 codec = new Base64();
+ for (String s : BASE64_IMPOSSIBLE_CASES) {
+ try {
+ codec.decode(s);
+ fail();
+ } catch (IllegalArgumentException ex) {
+ // expected
+ }
+ }
+ }
}
diff --git a/src/test/java/org/apache/commons/codec/binary/Base64TestData.java b/src/test/java/org/apache/commons/codec/binary/Base64TestData.java
index e54d829..3e53d3a 100644
--- a/src/test/java/org/apache/commons/codec/binary/Base64TestData.java
+++ b/src/test/java/org/apache/commons/codec/binary/Base64TestData.java
@@ -30,7 +30,7 @@ import java.util.Random;
*/
public class Base64TestData {
- public static final String CODEC_101_MULTIPLE_OF_3 = "123";
+ public static final String CODEC_101_MULTIPLE_OF_3 = "124";
public static final String CODEC_98_NPE
= "YWJjZGVmZ2hpamtsbW5vcHFyc3R1dnd4eXpBQkNERUZHSElKS0xNTk9QUVJTVFVWV1hZWjAxMjM";
diff --git a/src/test/java/org/apache/commons/codec/net/BCodecTest.java b/src/test/java/org/apache/commons/codec/net/BCodecTest.java
index f319152..f9b730c 100644
--- a/src/test/java/org/apache/commons/codec/net/BCodecTest.java
+++ b/src/test/java/org/apache/commons/codec/net/BCodecTest.java
@@ -33,6 +33,12 @@ import org.junit.Test;
*
*/
public class BCodecTest {
+ private static final String[] BASE64_IMPOSSIBLE_CASES = {
+ "ZE==",
+ "ZmC=",
+ "Zm9vYE==",
+ "Zm9vYmC=",
+ };
static final int SWISS_GERMAN_STUFF_UNICODE[] =
{ 0x47, 0x72, 0xFC, 0x65, 0x7A, 0x69, 0x5F, 0x7A, 0xE4, 0x6D, 0xE4 };
@@ -147,4 +153,17 @@ public class BCodecTest {
// Exception expected, test segment passes.
}
}
+
+ @Test
+ public void testBase64ImpossibleSamples() {
+ BCodec codec = new BCodec();
+ for (String s : BASE64_IMPOSSIBLE_CASES) {
+ try {
+ codec.decode(s);
+ fail();
+ } catch (DecoderException ex) {
+ // expected
+ }
+ }
+ }
}