[
https://issues.apache.org/jira/browse/CODEC-134?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13222062#comment-13222062
]
Hanson Char commented on CODEC-134:
-----------------------------------
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
>
> 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
