[jira] [Commented] (CODEC-270) Base32 and Base64 still allow decoding some invalid trailing characters

["Alex Herbert (Jira)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/CODEC-270?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16981218#comment-16981218 ] 

Alex Herbert commented on CODEC-270:
------------------------------------

Fix is in PR #29.

When implementing the fix I noticed that Base64 ignores a single trailing base 64 character. This encodes only 6-bits and is not enough to create a byte. But for consistency should this:
{code:java}
            switch (context.modulus) {
//              case 0 : // impossible, as excluded above
                case 1 : // 6 bits - ignore entirely
                    // TODO not currently tested; perhaps it is impossible?
                    break;
{code}
be changed to:
{code:java}
            switch (context.modulus) {
//              case 0 : // impossible, as excluded above
                case 1 : // 6 bits = no bytes
                    validateCharacter(MASK_6BITS, context);
                    break;
{code}
This would then throw under all circumstances except if the single trailing digit encoded zero. Given that this is confusing (the '=' character is padding) I would instead just throw an exception:
{code:java}
            switch (context.modulus) {
//              case 0 : // impossible, as excluded above
                case 1 : // 6 bits = no bytes
                    throw new IllegalArgumentException("Single trailing character is not allowed");
                    break;
{code}
This would make the following impossible to decode by throwing:
{code:java}
String text = "A";
byte[] bytes = Base64.decodeBase64(text);
{code}
Currently it returns a zero length byte[] as this test currently passes:
{code:java}
@Test
public void decodeSmallString() {
    String text = "A";
    byte[] bytes = Base64.decodeBase64(text);
    Assert.assertArrayEquals(new byte[0], bytes);
}
{code}

> Base32 and Base64 still allow decoding some invalid trailing characters
> -----------------------------------------------------------------------
>
>                 Key: CODEC-270
>                 URL: https://issues.apache.org/jira/browse/CODEC-270
>             Project: Commons Codec
>          Issue Type: Bug
>    Affects Versions: 1.13
>            Reporter: Alex Herbert
>            Assignee: Alex Herbert
>            Priority: Minor
>          Time Spent: 10m
>  Remaining Estimate: 0h
>
> Both Base32 and Base64 check that the final bits from the trailing digit that will be discarded are zero.
> The test for the trailing bits in the final digits in Base64 is:
> {code:java}
> private long validateCharacter(final int numBitsToDrop, final Context context) {
>     if ((context.ibitWorkArea & numBitsToDrop) != 0) {
> {code}
> It should be:
> {code:java}
> private long validateCharacter(final int numBitsToDrop, final Context context) {
>     int mask = (1 << numBitsToDrop) - 1;
>     if ((context.ibitWorkArea & mask) != 0) {
> {code}
> Likewise in Base32.
> The following base64 is illegal but is still decoded:
> {noformat}
> AB==
> A : 000000
> B : 000001
> byte = 00000000 + 0001 discarded 
> {noformat}
> Here the check for the 4 trailing bits to drop in this case checks only bit 3 and ignores bit 1 which is set.
> Same for Base32, this is illegal:
> {noformat}
> AB======
> A : 00000
> B : 00001
> byte = 00000000 + 01 discarded
> {noformat}
> But the check for the 2 trailing bits to drop in this case checks bit 2 and ignores bit 1 which is set.
> Note: The test cases using "AC" has bit 2 set and so is flagged as invalid.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)