Polarssl Base64.c error check

Question

I used polarssl's base64.c file for decoding some data.

int base64_decode( unsigned char *dst, size_t *dlen,
                   const unsigned char *src, size_t slen ){
size_t i, n=0;
uint32_t j, x;
unsigned char *p;

for( i = n = j = 0; i < slen; i++ )
{
    if( ( slen - i ) >= 2 &&
        src[i] == '\r' && src[i + 1] == '\n' )
        continue;

    if( src[i] == '\n' )
        continue;

    if( src[i] == '=' && ++j > 2 )
        return( POLARSSL_ERR_BASE64_INVALID_CHARACTER );

    if( src[i] > 127 || base64_dec_map[src[i]] == 127 )
        return( POLARSSL_ERR_BASE64_INVALID_CHARACTER );

    if( base64_dec_map[src[i]] < 64 && j != 0 )
        return( POLARSSL_ERR_BASE64_INVALID_CHARACTER );

    n++;
}
n = ((n * 6) + 7) >> 3;
if( dst == NULL || *dlen < n )
{
    *dlen = n;
    return( POLARSSL_ERR_BASE64_BUFFER_TOO_SMALL );
}
for( j = 3, n = x = 0, p = dst; i > 0; i--, src++ ){
    if( *src == '\r' || *src == '\n' )
        continue;

    j -= ( base64_dec_map[*src] == 64 );
    x  = (x << 6) | ( base64_dec_map[*src] & 0x3F );

    if( ++n == 4 )
    {
        n = 0;
        if( j > 0 ) *p++ = (unsigned char)( x >> 16 );
        if( j > 1 ) *p++ = (unsigned char)( x >>  8 );
        if( j > 2 ) *p++ = (unsigned char)( x       );
    }
}

*dlen = p - dst;

return( 0 );
}

When my data comes to this part of the code 'n' comes bigger than '*dlen'; but when i make this part as comment lines function also works well.

if( dst == NULL || *dlen < n )
{
    *dlen = n;
    return( POLARSSL_ERR_BASE64_BUFFER_TOO_SMALL );
}

Is there any possibility this code part is unnecessary? I mean if i remove this part from function, would i be miss a detail in the bussiness logic?

Edit:

For example:

src : A7ViV8hpIon0lisFRCvQpw==

dlen: 18

dst: 3 b5 62 57 c8 69 22 89 f4 96 2b 5 44 2b d0 a7 0 0

actually last two zeros in 'dst' shouldn't be in correct result, when i send 'dlen' as 16 and make comment out that if statement, i get correct result. I think n calculation is not correct or i am missing something.

It protects you against buffer overruns, memory trashing and crashes caused by improperly initialized or unsuitable parameters, so it would be ***very*** unwise to remove that test. Why would you want to remove it? — fvu, Jul 02 '14 at 11:37
I've just edited my question with an example. I hope i can explain the situation. — nerd, Jul 02 '14 at 11:53
The routine seems to improperly handle the role of the padding characters to calculate the output length (see rysst's answer to http://stackoverflow.com/questions/342409/how-do-i-base64-encode-decode-in-c to see how it should be calculated). This looks like a bug and should be signaled to the software's maintainer. — fvu, Jul 02 '14 at 15:11
@fvu Thanks for getting us tipped! Fixed in the new [PolarSSL 1.3.8 release](https://polarssl.org/tech-updates/releases/polarssl-1.3.8-released) — Paul, Jul 11 '14 at 11:20

score 2 · Accepted Answer · answered Jul 04 '14 at 10:58

2

You are correct! There is a bug in the returned length (and length check) in base64_decode().

There is one line missing after:

n = ((n * 6) + 7) >> 3;

If you put:

n -= j;

directly after that, it will work as expected.

answered Jul 04 '14 at 10:58

Paul

1,049
9
10

i think j variable never changed. So "n -= j" does not effect n variable. What do you think about this outcome? – nerd Jul 04 '14 at 12:58
`j` changed in this line `if( src[i] == '=' && ++j > 2 )` when the `=` characters are parsed.. – Paul Jul 04 '14 at 16:28

Polarssl Base64.c error check

1 Answers1