[ 
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

        

Reply via email to