Skip to content

Commit

Permalink
base64dec: don't read out of bounds
Browse files Browse the repository at this point in the history
Previously, base64dec checked terminating input '\0' every 4 calls to
base64dec_getc, where the latter progressed one or more chars on each
call, and could read past '\0' in the way it was used.

The input to base64dec currently comes only from OSC 52 escape seq
(copy to clipboard), and reading past '\0' or even past the buffer
boundary was easy to trigger.

Also, even if we could trust external input to be valid base64, there
are different base64 standards, and not all of them require padding
to 4 bytes blocks (using trailing '=' chars).

It didn't affect short OSC 52 strings because the buffer is initialized
to 0's, so typically it did stop within the buffer, but if the string
was trimmed to fit (the buffer is 512 bytes) then it did also read past
the end of the buffer, and the decoded suffix ended up arbitrary.

This patch makes base64dec_getc not progress past '\0', and instead
produce fake trailing padding of '='.

Additionally, at base64dec, if padding is detected at the first or
second byte of a quartet, then we identify it as invalid and abort
(a valid quartet has at least two leading non-padding bytes).
  • Loading branch information
avih authored and hiltjo committed Nov 10, 2019
1 parent 8386642 commit ea4d933
Showing 1 changed file with 5 additions and 1 deletion.
6 changes: 5 additions & 1 deletion st.c
Original file line number Diff line number Diff line change
Expand Up @@ -366,7 +366,7 @@ char
base64dec_getc(const char **src)
{
while (**src && !isprint(**src)) (*src)++;
return *((*src)++);
return **src ? *((*src)++) : '='; /* emulate padding if string ends */
}

char *
Expand All @@ -384,6 +384,10 @@ base64dec(const char *src)
int c = base64_digits[(unsigned char) base64dec_getc(&src)];
int d = base64_digits[(unsigned char) base64dec_getc(&src)];

/* invalid input. 'a' can be -1, e.g. if src is "\n" (c-str) */
if (a == -1 || b == -1)
break;

*dst++ = (a << 2) | ((b & 0x30) >> 4);
if (c == -1)
break;
Expand Down

0 comments on commit ea4d933

Please sign in to comment.