Don't tolerate non-base64 chars in base64 stream
Hrvoje Niksic <[email protected]> Mon, 04 Jul 2005 15:30:21 +0200
| Newsgroups | gmane.comp.web.wget.patches |
|---|---|
| Message-ID | <[email protected]> |
Based on previous observation (http://tinyurl.com/aarq3), this patch modifies the base64 decoder to not tolerate random garbage in mid-stream. 2005-07-04 Hrvoje Niksic <[email protected]> * utils.c (base64_decode): Don't silently tolerate non-base64 non-white-space characters in the base64 stream. Index: src/utils.c =================================================================== --- src/utils.c (revision 1958) +++ src/utils.c (working copy) @@ -1899,16 +1899,24 @@ #define IS_ASCII(c) (((c) & 0x80) == 0) #define IS_BASE64(c) ((IS_ASCII (c) && base64_char_to_value[c] >= 0) || c == '=') -/* Get next character from the string, except that non-base64 - characters are ignored, as mandated by rfc2045. */ -#define NEXT_BASE64_CHAR(c, p) do { \ - c = *p++; \ -} while (c != '\0' && !IS_BASE64 (c)) +/* Get next character from the string, ignoring whitespace. C should + be int, and will contain the next character, \0 if end is reached, + or -1 if a non-ws non-base64 character is read. */ +#define NEXT_BASE64_CHAR(c, p) for (;;) { \ + c = (unsigned char) *p++; \ + if (IS_BASE64 (c) || c == '\0') \ + break; \ + else if (!ISSPACE (c)) \ + { \ + c = -1; \ + break; \ + } \ + /* c is whitespace, keep looping */ \ +} -/* Decode data from BASE64 (assumed to be encoded as base64) into - memory pointed to by TO. TO should be large enough to accomodate - the decoded data, which is guaranteed to be less than - strlen(base64). +/* Decode data from BASE64 (pointer to \0-terminated text) into memory + pointed to by TO. TO should be large enough to accomodate the + decoded data, which is guaranteed to be less than strlen(base64). Since TO is assumed to contain binary data, it is not NUL-terminated. The function returns the length of the data @@ -1920,7 +1928,7 @@ { /* Table of base64 values for first 128 characters. Note that this assumes ASCII (but so does Wget in other places). */ - static short base64_char_to_value[128] = + static signed char base64_char_to_value[128] = { -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, /* 0- 9 */ -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, /* 10- 19 */ @@ -1936,36 +1944,39 @@ 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, /* 110-119 */ 49, 50, 51, -1, -1, -1, -1, -1 /* 120-127 */ }; +#define BASE64_CHAR_TO_VALUE(c) ((int) base64_char_to_value[c]) const char *p = base64; char *q = to; while (1) { - unsigned char c; + int c; unsigned long value; /* Process first byte of a quadruplet. */ NEXT_BASE64_CHAR (c, p); if (!c) break; - if (c == '=') - return -1; /* illegal '=' while decoding base64 */ - value = base64_char_to_value[c] << 18; + if (c == '=' || c == -1) + return -1; /* illegal char while decoding base64 */ + value = BASE64_CHAR_TO_VALUE (c) << 18; /* Process scond byte of a quadruplet. */ NEXT_BASE64_CHAR (c, p); if (!c) return -1; /* premature EOF while decoding base64 */ - if (c == '=') - return -1; /* illegal `=' while decoding base64 */ - value |= base64_char_to_value[c] << 12; + if (c == '=' || c == -1) + return -1; /* illegal char while decoding base64 */ + value |= BASE64_CHAR_TO_VALUE (c) << 12; *q++ = value >> 16; /* Process third byte of a quadruplet. */ NEXT_BASE64_CHAR (c, p); if (!c) return -1; /* premature EOF while decoding base64 */ + if (c == -1) + return -1; /* illegal char while decoding base64 */ if (c == '=') { @@ -1977,7 +1988,7 @@ continue; } - value |= base64_char_to_value[c] << 6; + value |= BASE64_CHAR_TO_VALUE (c) << 6; *q++ = 0xff & value >> 8; /* Process fourth byte of a quadruplet. */ @@ -1986,10 +1997,13 @@ return -1; /* premature EOF while decoding base64 */ if (c == '=') continue; + if (c == -1) + return -1; /* illegal char while decoding base64 */ - value |= base64_char_to_value[c]; + value |= BASE64_CHAR_TO_VALUE (c); *q++ = 0xff & value; } +#undef BASE64_CHAR_TO_VALUE return q - to; }