[ https://issues.apache.org/jira/browse/CODEC-134?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Hanson Char updated CODEC-134: ------------------------------ Comment: was deleted (was: 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 a9da10f..9cd293b 100644 --- a/src/main/java/org/apache/commons/codec/binary/Base32.java +++ b/src/main/java/org/apache/commons/codec/binary/Base32.java @@ -274,7 +274,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. * * Output is written to {@link #buffer} as 8-bit octets, using {@link #pos} as the buffer position */ @@ -320,30 +320,30 @@ public class Base32 extends BaseNCodec { // we ignore partial bytes, i.e. only multiples of 8 count switch (modulus) { case 2 : // 10 bits, drop 2 and output one byte - buffer[pos++] = (byte) ((bitWorkArea >> 2) & MASK_8BITS); + buffer[pos++] = (byte) (dropBits(2) & MASK_8BITS); break; case 3 : // 15 bits, drop 7 and output 1 byte - buffer[pos++] = (byte) ((bitWorkArea >> 7) & MASK_8BITS); + buffer[pos++] = (byte) (dropBits(7) & MASK_8BITS); break; case 4 : // 20 bits = 2*8 + 4 - bitWorkArea = bitWorkArea >> 4; // drop 4 bits + bitWorkArea = dropBits(4); // drop 4 bits buffer[pos++] = (byte) ((bitWorkArea >> 8) & MASK_8BITS); buffer[pos++] = (byte) ((bitWorkArea) & MASK_8BITS); break; case 5 : // 25bits = 3*8 + 1 - bitWorkArea = bitWorkArea >> 1; + bitWorkArea = dropBits(1); buffer[pos++] = (byte) ((bitWorkArea >> 16) & MASK_8BITS); buffer[pos++] = (byte) ((bitWorkArea >> 8) & MASK_8BITS); buffer[pos++] = (byte) ((bitWorkArea) & MASK_8BITS); break; case 6 : // 30bits = 3*8 + 6 - bitWorkArea = bitWorkArea >> 6; + bitWorkArea = dropBits(6); buffer[pos++] = (byte) ((bitWorkArea >> 16) & MASK_8BITS); buffer[pos++] = (byte) ((bitWorkArea >> 8) & MASK_8BITS); buffer[pos++] = (byte) ((bitWorkArea) & MASK_8BITS); break; case 7 : // 35 = 4*8 +3 - bitWorkArea = bitWorkArea >> 3; + bitWorkArea = dropBits(3); buffer[pos++] = (byte) ((bitWorkArea >> 24) & MASK_8BITS); buffer[pos++] = (byte) ((bitWorkArea >> 16) & MASK_8BITS); buffer[pos++] = (byte) ((bitWorkArea >> 8) & MASK_8BITS); @@ -352,6 +352,28 @@ public class Base32 extends BaseNCodec { } } } + + /** + * <p> + * Drops the specified number of least significant bits from the + * {@link #bitWorkArea}. + * </p> + * + * @param numBitsToDrop + * number of least significant bits to drop + * + * @return the value of {@link #bitWorkArea} after dropping the + * specified number of least significant bits + * + * @throws IllegalArgumentException + * if the bits being dropped contain any non-zero value + */ + private long dropBits(int numBitsToDrop) { + if ((bitWorkArea & numBitsToDrop) != 0) { + throw new IllegalArgumentException("Last encoded character (before the paddings if any) is a valid base 32 alphabet but not a possible value"); + } + return bitWorkArea >> numBitsToDrop; + } /** * <p> 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 1ee0eba..4261f88 100644 --- a/src/main/java/org/apache/commons/codec/binary/Base64.java +++ b/src/main/java/org/apache/commons/codec/binary/Base64.java @@ -410,7 +410,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. */ @Override void decode(byte[] in, int inPos, int inAvail) { @@ -455,17 +455,39 @@ public class Base64 extends BaseNCodec { // case 1: // 6 bits - ignore entirely // break; case 2 : // 12 bits = 8 + 4 - bitWorkArea = bitWorkArea >> 4; // dump the extra 4 bits + bitWorkArea = dropBits(4); // drop the extra 4 bits buffer[pos++] = (byte) ((bitWorkArea) & MASK_8BITS); break; case 3 : // 18 bits = 8 + 8 + 2 - bitWorkArea = bitWorkArea >> 2; // dump 2 bits + bitWorkArea = dropBits(2); // drop 2 bits buffer[pos++] = (byte) ((bitWorkArea >> 8) & MASK_8BITS); buffer[pos++] = (byte) ((bitWorkArea) & MASK_8BITS); break; } } } + + /** + * <p> + * Drops the specified number of least significant bits from the + * {@link #bitWorkArea}. + * </p> + * + * @param numBitsToDrop + * number of least significant bits to drop + * + * @return the value of {@link #bitWorkArea} after dropping the + * specified number of least significant bits + * + * @throws IllegalArgumentException + * if the bits being dropped contain any non-zero value + */ + private int dropBits(int numBitsToDrop) { + if ((bitWorkArea & 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 bitWorkArea >> numBitsToDrop; + } /** * Tests a given byte array to see if it contains only valid characters within the Base64 alphabet. Currently the ) > Base32 would decode some invalid Base32 encoded string into arbitrary value > --------------------------------------------------------------------------- > > Key: CODEC-134 > URL: https://issues.apache.org/jira/browse/CODEC-134 > Project: Commons Codec > Issue Type: Bug > Affects Versions: 1.6 > Environment: All > Reporter: Hanson Char > Labels: security > Attachments: patch.txt > > > Example, there is no byte array value that can be encoded into the string > "C5CYMIHWQUUZMKUGZHGEOSJSQDE4L===", but the existing Base32 implementation > would not reject it but decode it into an arbitrary value which if re-encoded > again using the same implementation would result in the string > "C5CYMIHWQUUZMKUGZHGEOSJSQDE4K===". > Instead of blindly decoding the invalid string, the Base32 codec should > reject it (eg by throwing IlleglArgumentException) to avoid security > exploitation (such as tunneling additional information via seemingly valid > base 32 strings). -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira