The function
int
b64_pton(char const *src, u_char *target, size_t targsize);
in src/lib/libc/net/base64.c has a small bug with no consequences in
the rest of the tree.
By design, b64_pton() can be called with a NULL pointer as `target', in
which case it calculates the number of bytes encoded in the Base64
string `src'. However, in presence of padding, the function doesn't
fully check whether the string is a valid Base64 string: It only counts
the number of padding characters and checks whether they occur in the
correct positions.
For instance, `b64_pton("//==", NULL, 1);' returns 1, even though "//=="
isn't actually a valid Base64 string.
The call `b64_pton("//==", target, 1);' correctly rejects the string
and returns -1.
Similarly, `b64_pton("///=", NULL, 2);' shouldn't return 2, but -1.
To fix this, I suggest to move the calculation of `nextbyte' in
states 1 and 2 out of the conditional `if (target)' and to use
`nextbyte' as a flag whether padding is allowed or not -- the current
character is allowed to be a padding character if and only if
(nextbyte == 0 && (state == 2 || state == 3)).
A nice side effect is that
if (target && tarindex < targsize && target[tarindex] != 0)
at the end of the routine can be replaced by the much simpler
if (nextbyte)
It makes more sense to me to bail out right away rather than deferring
this check to the end of the fiddling with the padding characters.
Index: base64.c
===================================================================
RCS file: /cvs/src/lib/libc/net/base64.c,v
retrieving revision 1.7
diff -u -p -r1.7 base64.c
--- base64.c 31 Dec 2013 02:32:56 -0000 1.7
+++ base64.c 16 Sep 2014 12:51:34 -0000
@@ -197,6 +197,7 @@ b64_pton(src, target, targsize)
u_char nextbyte;
char *pos;
+ nextbyte = 0;
state = 0;
tarindex = 0;
@@ -204,8 +205,17 @@ b64_pton(src, target, targsize)
if (isspace(ch)) /* Skip whitespace anywhere. */
continue;
- if (ch == Pad64)
+ if (ch == Pad64) {
+ if (nextbyte)
+ /*
+ * We're in state 2 or 3 and the last character
+ * slopped over the last byte boundary into the
+ * padded byte.
+ */
+ return (-1);
+
break;
+ }
pos = strchr(Base64, ch);
if (pos == 0) /* A non-base64 character. */
@@ -221,11 +231,11 @@ b64_pton(src, target, targsize)
state = 1;
break;
case 1:
+ nextbyte = ((pos - Base64) & 0x0f) << 4;
if (target) {
if (tarindex >= targsize)
return (-1);
target[tarindex] |= (pos - Base64) >> 4;
- nextbyte = ((pos - Base64) & 0x0f) << 4;
if (tarindex + 1 < targsize)
target[tarindex+1] = nextbyte;
else if (nextbyte)
@@ -235,11 +245,11 @@ b64_pton(src, target, targsize)
state = 2;
break;
case 2:
+ nextbyte = ((pos - Base64) & 0x03) << 6;
if (target) {
if (tarindex >= targsize)
return (-1);
target[tarindex] |= (pos - Base64) >> 2;
- nextbyte = ((pos - Base64) & 0x03) << 6;
if (tarindex + 1 < targsize)
target[tarindex+1] = nextbyte;
else if (nextbyte)
@@ -249,6 +259,7 @@ b64_pton(src, target, targsize)
state = 3;
break;
case 3:
+ nextbyte = 0;
if (target) {
if (tarindex >= targsize)
return (-1);
@@ -292,16 +303,6 @@ b64_pton(src, target, targsize)
for (; ch != '\0'; ch = (unsigned char)*src++)
if (!isspace(ch))
return (-1);
-
- /*
- * Now make sure for cases 2 and 3 that the "extra"
- * bits that slopped past the last full byte were
- * zeros. If we don't check them, they become a
- * subliminal channel.
- */
- if (target && tarindex < targsize &&
- target[tarindex] != 0)
- return (-1);
}
} else {
/*
