Re: [gmime-devel] Bug in base64 decoding - corrupted last bytes of data

[Jeffrey Stedfast <fejj novell com>]

On 08/30/2010 09:17 AM, Damian Pietras wrote:
> On Mon, Aug 30, 2010 at 08:59:46AM -0400, Jeffrey Stedfast wrote:
>   
>> On 08/30/2010 04:58 AM, Damian Pietras wrote:
>>     
>>> The code that does base64 decoding
>>> (g_mime_encoding_base64_decode_step()) has a bug that leads to
>>> corruption of last bytes of data in some situations.
>>>   
>>>       
>> Hi Damian,
>>
>> Do you have an example base64 stream I can use to test fixes on? I'd
>> prefer not to break the API.
>>     
> I'm attaching an example. The data is strange (one char per line) but
> valid. It must be exactly this input and the buffer size like in the code.
> Other combinations are also possible, but this is the smallest example I
> have.
>
>   

Thanks. I've attached another possible patch which doesn't break API.
Lemme know if that works for you.

Jeff

diff --git a/gmime/gmime-encodings.c b/gmime/gmime-encodings.c
index 8880592..8acbf6f 100644
--- a/gmime/gmime-encodings.c
+++ b/gmime/gmime-encodings.c
@@ -506,45 +506,60 @@ g_mime_encoding_base64_decode_step (const unsigned char *inbuf, size_t inlen, un
 	const unsigned char *inend;
 	register guint32 saved;
 	unsigned char c;
-	int i;
+	int npad, n, i;
 	
 	inend = inbuf + inlen;
 	outptr = outbuf;
+	inptr = inbuf;
 	
-	/* convert 4 base64 bytes to 3 normal bytes */
+	npad = (*state >> 8) & 0xff;
+	n = *state & 0xff;
 	saved = *save;
-	i = *state;
-	inptr = inbuf;
+	
+	/* convert 4 base64 bytes to 3 normal bytes */
 	while (inptr < inend) {
 		c = gmime_base64_rank[*inptr++];
 		if (c != 0xff) {
 			saved = (saved << 6) | c;
-			i++;
-			if (i == 4) {
+			n++;
+			if (n == 4) {
 				*outptr++ = saved >> 16;
 				*outptr++ = saved >> 8;
 				*outptr++ = saved;
-				i = 0;
+				n = 0;
+				
+				if (npad > 0) {
+					outptr -= npad;
+					npad = 0;
+				}
 			}
 		}
 	}
 	
-	*save = saved;
-	*state = i;
-	
-	/* quick scan back for '=' on the end somewhere */
-	/* fortunately we can drop 1 output char for each trailing = (upto 2) */
-	i = 2;
-	while (inptr > inbuf && i) {
+	/* quickly scan back for '=' on the end somewhere */
+	/* fortunately we can drop 1 output char for each trailing '=' (up to 2) */
+	for (i = 2; inptr > inbuf && i; ) {
 		inptr--;
 		if (gmime_base64_rank[*inptr] != 0xff) {
-			if (*inptr == '=' && outptr > outbuf)
-				outptr--;
+			if (*inptr == '=' && outptr > outbuf) {
+				if (n == 0) {
+					/* we've got a complete quartet so it's
+					   safe to drop an output character. */
+					outptr--;
+				} else if (npad < 2) {
+					/* keep a record of the number of ='s at
+					   the end of the input stream, up to 2 */
+					npad++;
+				}
+			}
+			
 			i--;
 		}
 	}
 	
-	/* if i != 0 then there is a truncation error! */
+	*state = (npad << 8) | n;
+	*save = saved;
+	
 	return (outptr - outbuf);
 }