[Home]

Summary:ASTERISK-07188: [patch] Regression issue: ast_base64encode requires larger buffer than before
Reporter:mikma (mikma)Labels:
Date Opened:2006-06-16 10:10:51Date Closed:2006-08-25 12:49:02
Priority:MinorRegression?No
Status:Closed/CompleteComponents:Core/General
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) asterisk-trunk-r34464-base64encode.patch
Description:I have found a regression issue in ast_base64encode and ast_base64encode_full. Both functions require a buffer containing one character more than are actually written to the buffer.

This is a changed behaviour which should be documented if it's disired, otherwise it's a regression bug.
Comments:By: Tilghman Lesher (tilghman) 2006-06-22 11:26:51

You're mistaken.  Remember that we start numbering from 0, so an allocation of "char foo[10]" actually has its maximum array index at foo[9].  Your patch would create a possible buffer overrun.
By: mikma (mikma) 2006-06-22 13:10:56

Assume srclen=3 and max=5. A destination buffer of 5 chars should be enough to store the base64 encoding of 3 bytes including null char at the end.

But when the following if-statment is executed, max has already been decreased to 4 to reserve space for the null byte. And the then-statement isn't executed since cnt is 0 and cnt + 4 < max is false.

             if ((bits == 24) && (cnt + 4 < max)) {
                       *dst++ = base64[(byte >> 18) & 0x3f];
                       *dst++ = base64[(byte >> 12) & 0x3f];
                       *dst++ = base64[(byte >> 6) & 0x3f];
                       *dst++ = base64[byte & 0x3f];
                       cnt += 4;
                       col += 4;
                       bits = 0;
                       byte = 0;
             }
By: Clod Patry (junky) 2006-08-08 22:05:41

mikma: could you explain how you did ur regression test exactly?
just a looping of BASE64_ENCODE/DECODE?
By: mikma (mikma) 2006-08-14 04:56:14

I found the regression bug since it broke the securertp-trunk[1]. The patch has been applied to the branch.

[1]http://svn.digium.com/svn/asterisk/team/oej/securertp-trunk
By: Tilghman Lesher (tilghman) 2006-08-25 12:49:02

Fixed in trunk.