diff --git a/src/changes/changes.xml b/src/changes/changes.xml index 18a6cf4a13..357bae99ca 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -45,6 +45,7 @@ The type attribute can be add,update,fix,remove. Update from Java 7 to Java 8 + Reject any decode request for a value that is impossible to encode to for Base32/Base64 rather than blindly decoding. 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 22c1d65fe7..3301ed4558 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 Base32(final int lineLength, final byte[] lineSeparator, final boolean us * @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 @@ void decode(final byte[] in, int inPos, final int inAvail, final Context context // 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 @@ void encode(final byte[] in, int inPos, final int inAvail, final Context context public boolean isInAlphabet(final byte octet) { return octet >= 0 && octet < decodeTable.length && decodeTable[octet] != -1; } + + /** + *

+ * Validates whether the character is possible in the context of the set of possible base 32 values. + *

+ * + * @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 3641c5cdff..06e2db73bd 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 @@ void encode(final byte[] in, int inPos, final int inAvail, final Context context * @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 @@ void decode(final byte[] in, int inPos, final int inAvail, final Context context // 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 @@ protected boolean isInAlphabet(final byte octet) { return octet >= 0 && octet < decodeTable.length && decodeTable[octet] != -1; } + /** + *

+ * Validates whether the character is possible in the context of the set of possible base 64 values. + *

+ * + * @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 5940f8f211..cdaedee8d7 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 String decode(final String value) throws DecoderException { } 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 734b8bbcf5..547c241f6a 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 void testSingleCharEncoding() { } } + @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 4206cab02b..5bec44e45d 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 void testHugeLineSeparator() { 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 e54d8294ff..3e53d3a4ff 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 @@ */ 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 f31915224a..f9b730c749 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 @@ * */ 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 void testDecodeObjects() throws Exception { // 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 + } + } + } }