Improving the BASE64 decoder to handle lines that are not multiples of four

The BASE64 decoder didn't take into account lines that were not
multiples of four and this resulted in a corrupted decode. Changed
the decoder so that state can be preserved between subsequent calls.
Manual merge of PR#99
Closes #96

Author: jkbzh

Changelog

@@ -101,6 +101,13 @@ HYPERMAIL VERSION 3.0.0
 	print modules are used to, but including now markers for the beginning
 	and end of sections, etc.
 
+2023-05-16 Andy Valencia (@vandys)
+	* src/{parse.c, base64.h, base64.c}
+	The BASE64 decoder didn't take into account lines that were not
+	multiples of four and this resulted in a corrupted decode. Changed the
+	decoder so that state can be preserved between subsequent calls.
+	Closes #96
+
 2023-05-12 @shlomif
 	* README
 	Fixed spelling errors

src/base64.c

@@ -10,85 +10,146 @@
 **
 ** - Encoded strings that ended with more than one = caused the decode 
 **   function+ to generate 3 extra zero bytes at the end of the output.
+**
+** CHANGES by Andy Valencia - May 15 2023:
+**
+** - Preserve decoding state between calls to the
+**   base64_stream_decode() function to take into account UA that
+**   output intermediate base64 lines that are not always multiple of
+**   four.
 */
 
 #include "hypermail.h"
 #include "base64.h"
 
-void base64Decode(char *intext, char *out, int *length)
+/*
+ * base64_decoder_state_new()
+ *	Allocate a new base64 decoder state
+ */
+struct base64_decoder_state *base64_decoder_state_new(void)
+{
+    struct base64_decoder_state *st = (struct base64_decoder_state *)emalloc(sizeof(struct base64_decoder_state));
+    if (!st) {
+	return(0);
+    }
+    memset(st, 0, sizeof(struct base64_decoder_state));
+    return(st);
+}
+
+/*
+ * base64_decoder_state_free()
+ *	Release storage
+ */
+void base64_decoder_state_free(struct base64_decoder_state *st)
+{
+    free(st);
+}
+
+/*
+ * base64_decode_stream()
+ *
+ * Accept base64 "intext", 
+ * place resulting decoded output in a null-terminated "out".
+ *
+ *  "st" is our state, which will be updated and can carry state between
+ *  calls.
+ */
+int base64_decode_stream(struct base64_decoder_state *st, const char *intext, char *out)
 {
-    unsigned char ibuf[4];
-    unsigned char obuf[3];
     char ignore;
-    char endtext = FALSE;
     char ch;
-    int lindex = 0;
-    *length = 0;
-
-    memset(ibuf, 0, sizeof(ibuf));
+    int length;
+    
+    /* Ignore trailing garbage */
+    if (st->endtext) {
+	*out = '\0';
+	return(0);
+    }
 
+    length = 0;
     while (*intext) {
 	ch = *intext;
-
 	ignore = FALSE;
-	if ((ch >= 'A') && (ch <= 'Z'))
+        
+	if ((ch >= 'A') && (ch <= 'Z')) {
 	    ch = ch - 'A';
-	else if ((ch >= 'a') && (ch <= 'z'))
+        } else if ((ch >= 'a') && (ch <= 'z')) {
 	    ch = ch - 'a' + 26;
-	else if ((ch >= '0') && (ch <= '9'))
+        } else if ((ch >= '0') && (ch <= '9')) {
 	    ch = ch - '0' + 52;
-	else if (ch == '+')
+        } else if (ch == '+') {
 	    ch = 62;
-	else if (ch == '=') {	/* end of text */
-	    if (endtext)
+        } else if (ch == '=') {	/* end of text */
+	    if (st->endtext) {
 		break;
-	    endtext = TRUE;
-	    lindex--;
-	    if (lindex < 0)
-		lindex = 3;
-	}
-	else if (ch == '/')
+            }
+	    st->endtext = TRUE;
+	    st->lindex--;
+	    if (st->lindex < 0) {
+		st->lindex = 3;
+            }
+	} else if (ch == '/') {
 	    ch = 63;
-	else if (endtext)
+        } else if (st->endtext) {
 	    break;
-	else
+        } else {
 	    ignore = TRUE;
-
+        }
+        
 	if (!ignore) {
-	    if (!endtext) {
-		ibuf[lindex] = ch;
+	    if (!st->endtext) {
+		st->ibuf[st->lindex] = ch;
 
-		lindex++;
-		lindex &= 3;	/* use bit arithmetic instead of remainder */
+		st->lindex++;
+		st->lindex &= 3;	/* use bit arithmetic instead of remainder */
 	    }
-	    if ((0 == lindex) || endtext) {
-
-		obuf[0] = (ibuf[0] << 2) | ((ibuf[1] & 0x30) >> 4);
-		obuf[1] =
-		    ((ibuf[1] & 0x0F) << 4) | ((ibuf[2] & 0x3C) >> 2);
-		obuf[2] = ((ibuf[2] & 0x03) << 6) | (ibuf[3] & 0x3F);
+            if ((0 == st->lindex) || st->endtext) {
 
-		switch (lindex) {
+		st->obuf[0] = (st->ibuf[0] << 2) | ((st->ibuf[1] & 0x30) >> 4);
+		st->obuf[1] =
+		    ((st->ibuf[1] & 0x0F) << 4) | ((st->ibuf[2] & 0x3C) >> 2);
+		st->obuf[2] = ((st->ibuf[2] & 0x03) << 6) | (st->ibuf[3] & 0x3F);
+                
+		switch (st->lindex) {
 		case 1:
-		    sprintf(out, "%c", obuf[0]);
-		    out++;
-		    (*length)++;
+		    *out++ = st->obuf[0];
+		    length += 1;
 		    break;
 		case 2:
-		    sprintf(out, "%c%c", obuf[0], obuf[1]);
-		    out += 2;
-		    (*length) += 2;
+		    *out++ = st->obuf[0];
+		    *out++ = st->obuf[1];
+		    length += 2;
 		    break;
 		default:
-		    sprintf(out, "%c%c%c", obuf[0], obuf[1], obuf[2]);
-		    out += 3;
-		    (*length) += 3;
+		    *out++ = st->obuf[0];
+		    *out++ = st->obuf[1];
+		    *out++ = st->obuf[2];
+		    length += 3;
 		    break;
 		}
-		memset(ibuf, 0, sizeof(ibuf));
+		memset(st->ibuf, 0, sizeof(st->ibuf));
 	    }
 	}
 	intext++;
     }
     *out = 0;
+    return (length);
+}
+
+/*
+ * base64_decode_string()
+ *	Convenience wrapper when decoding a single string
+ */
+int base64_decode_string(const char *intext, char *out)
+{
+    struct base64_decoder_state *st = base64_decoder_state_new();
+    int length = 0;
+    
+    if (!st) {
+	return 0;
+    }
+    length = base64_decode_stream(st, intext, out);
+    base64_decoder_state_free(st);
+
+    return (length);
 }

src/base64.h

@@ -2,8 +2,25 @@
 ** MIME Decode - base64.c
 */
 
-#ifndef HYPERMAIL_BASE64_H
-#define HYPERMAIL_BASE64_H
-void base64Decode(char *, char *, int *);
+#ifndef _HYPERMAIL_BASE64_H
+#define _HYPERMAIL_BASE64_H
 
-#endif /* HYPERMAIL_BASE64_H */
+/* Common state as you feed successive lines into base64_decode_stream() */
+struct base64_decoder_state {
+    unsigned char ibuf[4]; /* input buffer */
+    unsigned char obuf[3]; /* output buffer */
+    int lindex;            /* index for ibuf / obuf */    
+    char endtext;          /* base64 end detected */
+};
+
+/* allocate and free a base64_state structure */
+struct base64_decoder_state *base64_decoder_state_new(void);
+void base64_decoder_state_free(struct base64_decoder_state *);
+
+/* decode a stream made of multiple base64 lines */
+int base64_decode_stream(struct base64_decoder_state *, const char *, char *);
+
+/* decode a single string */
+int base64_decode_string(const char *, char *);
+
+#endif /* _HYPERMAIL_BASE64_H */

src/parse.c

@@ -1019,12 +1019,12 @@ static char *mdecodeRFC2047(char *string, int length, char *charsetsave)
 	    }
 	    else if (!strcasecmp("b", encoding)) {
 		/* base64 decoding */
-		int len;
 	        size_t charsetlen;
 #ifdef HAVE_ICONV
                 size_t tmplen;
 		char *output2;
-		base64Decode(ptr, output, &len);
+                
+		base64_decode_string(ptr, output);
 		output2=i18n_convstring(output,charset,"UTF-8",&tmplen);
 		memcpy(output,output2,tmplen);
 		output += tmplen;
@@ -1033,7 +1033,9 @@ static char *mdecodeRFC2047(char *string, int length, char *charsetsave)
 		memcpy(charsetsave,charset,charsetlen);
 		charsetsave[charsetlen] = '\0';
 #else
-		base64Decode(ptr, output, &len);
+                int len;
+                
+		len = base64_decode_string(ptr, output)
 		output += len;
 #endif
 	    }
@@ -1754,7 +1756,8 @@ int parsemail(char *mbox,	/* file name */
 				   the content_disposition */
     char *description = NULL;	/* user-supplied description for an attachment */
     char attach_force;
-
+    struct base64_decoder_state *b64_decoder_state = NULL; /* multi-line base64 decoding */
+    
     EncodeType decode = ENCODE_NORMAL;
     ContentType content = CONTENT_TEXT;
 
@@ -2043,10 +2046,15 @@ int parsemail(char *mbox,	/* file name */
                         if (!message_headers_parsed) {
                             getname(head->line, &namep, &emailp);
                             if (set_spamprotect) {
-                                emailp = spamify(strsav(emailp));
+                                char *tmp;
+                                tmp = emailp;
+                                emailp = spamify(tmp);
+                                free(tmp);
                                 /* we need to "fix" the name as well, as sometimes
                                    the email ends up in the name part */
-                                namep = spamify(strsav(namep));
+                                tmp = strsav(namep);
+                                namep = spamify(tmp);
+                                free(tmp);
                             }
                         }
 		    }
@@ -2402,6 +2410,7 @@ int parsemail(char *mbox,	/* file name */
 			}
 			else if (!strncasecmp(ptr, "BASE64", 6)) {
 			    decode = ENCODE_BASE64;
+                            b64_decoder_state = base64_decoder_state_new();
 			}
 			else if (!strncasecmp(ptr, "8BIT", 4)) {
 			    decode = ENCODE_NORMAL;
@@ -3237,6 +3246,10 @@ int parsemail(char *mbox,	/* file name */
 		/* go back to default mode: */
                 file_created = alternative_lastfile_created = NO_FILE;
 		content = CONTENT_TEXT;
+                if (decode == ENCODE_BASE64) {
+                    base64_decoder_state_free(b64_decoder_state);
+                    b64_decoder_state = NULL;
+                }                                
 		decode = ENCODE_NORMAL;
 		Mime_B = FALSE;
                 skip_mime_epilogue = FALSE;
@@ -3532,6 +3545,10 @@ int parsemail(char *mbox,	/* file name */
 			}
 			/* go back to the MIME attachment default mode */
 			content = CONTENT_TEXT;
+                        if (decode == ENCODE_BASE64) {
+                            base64_decoder_state_free(b64_decoder_state);
+                            b64_decoder_state = NULL;
+                        }                                              
 			decode = ENCODE_NORMAL;
 			multilinenoend = FALSE;
                         *attachname = '\0';
@@ -3573,7 +3590,7 @@ int parsemail(char *mbox,	/* file name */
 		    }
 		    break;
 		case ENCODE_BASE64:
-		    base64Decode(line, newbuffer, &datalen);
+                    datalen = base64_decode_stream(b64_decoder_state, line, newbuffer);
 		    data = newbuffer;
 		    break;
 		case ENCODE_UUENCODE:
@@ -4003,8 +4020,9 @@ int parsemail(char *mbox,	/* file name */
 		    }
 		}
 
-		if (ENCODE_QP == decode)
+		if (ENCODE_QP == decode) {
 		    free(data);	/* this was allocatd by mdecodeQP() */
+                }
 	    }
 	}
     }
@@ -4221,6 +4239,10 @@ int parsemail(char *mbox,	/* file name */
 
 	/* go back to default mode: */
 	content = CONTENT_TEXT;
+        if (ENCODE_BASE64 == decode) {
+            base64_decoder_state_free(b64_decoder_state);
+            b64_decoder_state = NULL;
+        }                              
 	decode = ENCODE_NORMAL;
 	Mime_B = FALSE;
         skip_mime_epilogue = FALSE;