From: Andre Noll Date: Sun, 26 Apr 2015 21:41:00 +0000 (+0200) Subject: base64: Saner semantics for base64_decode() and uudecode(). X-Git-Tag: v0.5.7~20^2~5 X-Git-Url: http://git.tuebingen.mpg.de/?p=paraslash.git;a=commitdiff_plain;h=35fcd604e2ec04f8667c85fa1552932b1a0e6efb base64: Saner semantics for base64_decode() and uudecode(). Currently the callers of these functions must allocate a suitably sized buffer for the decoded data. It is easier to let the decoders allocate the result buffer, as implemented in this commit. The callers in crypt.c and gcrypt.c are adjusted accordingly. --- diff --git a/base64.c b/base64.c index e9892244..4809a2f2 100644 --- a/base64.c +++ b/base64.c @@ -17,29 +17,47 @@ static const char Base64[] = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/"; static const char Pad64 = '='; +/** Maximal possible size of the decoded data. */ +#define BASE64_MAX_DECODED_SIZE(_encoded_size) ((_encoded_size) / 4 * 3) + /** * base64-decode a buffer. * * \param src The buffer to decode. - * \param target Result is stored here. - * \param targsize Number of bytes of \a target. + * \param encoded_size The special value -1 means: look for terminating zero byte. + * \param result Points to dynamically allocated target buffer on success. + * \param decoded_size Number of bytes written to \a result. * * Skips all whitespace anywhere. Converts characters, four at a time, starting * at (or after) src from base - 64 numbers into three 8 bit bytes in the * target area. * - * \return The number of data bytes stored at the target, -E_BASE64 on errors. + * It is OK to pass a \p NULL pointer as \a decoded_size. The result is + * terminated with a zero byte. + * + * \return Standard. The contents of result \a and \a decoded_size are + * undefined on failure. */ -int base64_decode(char const *src, unsigned char *target, size_t targsize) +int base64_decode(char const *src, size_t encoded_size, char **result, + size_t *decoded_size) { unsigned int tarindex, state; int ch; char *pos; + const char *end = src + encoded_size; + unsigned char *target; + size_t targsize; + + if (encoded_size == (size_t)-1) + encoded_size = strlen(src); + targsize = BASE64_MAX_DECODED_SIZE(encoded_size); + target = para_malloc(targsize + 1); state = 0; tarindex = 0; - while ((ch = *src++) != '\0') { + while (src < end) { + ch = *src++; if (para_isspace(ch)) /* Skip whitespace anywhere. */ continue; @@ -48,18 +66,18 @@ int base64_decode(char const *src, unsigned char *target, size_t targsize) pos = strchr(Base64, ch); if (pos == NULL) /* A non-base64 character. */ - return -E_BASE64; + goto fail; switch (state) { case 0: if (tarindex >= targsize) - return -E_BASE64; + goto fail; target[tarindex] = (pos - Base64) << 2; state = 1; break; case 1: if (tarindex + 1 >= targsize) - return -E_BASE64; + goto fail; target[tarindex] |= (pos - Base64) >> 4; target[tarindex + 1] = ((pos - Base64) & 0x0f) << 4; tarindex++; @@ -67,7 +85,7 @@ int base64_decode(char const *src, unsigned char *target, size_t targsize) break; case 2: if (tarindex + 1 >= targsize) - return -E_BASE64; + goto fail; target[tarindex] |= (pos - Base64) >> 2; target[tarindex + 1] = ((pos - Base64) & 0x03) << 6; tarindex++; @@ -75,7 +93,7 @@ int base64_decode(char const *src, unsigned char *target, size_t targsize) break; case 3: if (tarindex >= targsize) - return -E_BASE64; + goto fail; target[tarindex] |= pos - Base64; tarindex++; state = 0; @@ -88,12 +106,12 @@ int base64_decode(char const *src, unsigned char *target, size_t targsize) * on a byte boundary, and/or with erroneous trailing characters. */ - if (ch == Pad64) { /* We got a pad char. */ + if (*src == Pad64) { /* We got a pad char. */ ch = *src++; /* Skip it, get next. */ switch (state) { case 0: /* Invalid = in first position */ case 1: /* Invalid = in second position */ - return -E_BASE64; + goto fail; case 2: /* Valid, means one byte of info */ /* Skip any number of spaces. */ @@ -102,7 +120,7 @@ int base64_decode(char const *src, unsigned char *target, size_t targsize) break; /* Make sure there is another trailing = sign. */ if (ch != Pad64) - return -E_BASE64; + goto fail; ch = *src++; /* Skip the = */ /* Fall through to "single trailing =" case. */ /* FALLTHROUGH */ @@ -114,7 +132,7 @@ int base64_decode(char const *src, unsigned char *target, size_t targsize) */ for (; ch != '\0'; ch = *src++) if (!isspace(ch)) - return -E_BASE64; + goto fail; /* * Now make sure for cases 2 and 3 that the "extra" @@ -123,7 +141,7 @@ int base64_decode(char const *src, unsigned char *target, size_t targsize) * subliminal channel. */ if (target[tarindex] != 0) - return -E_BASE64; + goto fail; } } else { /* @@ -131,37 +149,44 @@ int base64_decode(char const *src, unsigned char *target, size_t targsize) * have no partial bytes lying around. */ if (state != 0) - return -E_BASE64; + goto fail; } - return tarindex; + /* success */ + target[tarindex] = '\0'; /* just to be sure */ + *result = (char *)target; + if (decoded_size) + *decoded_size = tarindex; + return 1; +fail: + free(target); + return -E_BASE64; } /** - * uudecode a buffer. + * Decode a buffer using the uuencode Base64 algorithm. * * \param src The buffer to decode. - * \param target Result buffer. - * \param targsize The length of \a target in bytes. + * \param encoded_size Number of input bytes in the source buffer. + * \param result Contains the decoded data on success. + * \param decoded_size Number of output bytes on success. + * + * This is just a simple wrapper for \ref base64_decode() which strips + * whitespace. * - * This is just a simple wrapper for base64_decode() which strips whitespace. + * \return The return value of the underlying call to \ref base64_decode(). * - * \return The return value of the underlying call to base64_decode(). + * \sa uuencode(1), uudecode(1). */ -int uudecode(const char *src, unsigned char *target, size_t targsize) +int uudecode(char const *src, size_t encoded_size, char **result, + size_t *decoded_size) { - int len; - char *encoded, *p; + const char *end = src + encoded_size, *p; - /* copy the 'readonly' source */ - encoded = para_strdup(src); /* skip whitespace and data */ - for (p = encoded; *p == ' ' || *p == '\t'; p++) + for (p = src; p < end && (*p == ' ' || *p == '\t'); p++) ; - for (; *p != '\0' && *p != ' ' && *p != '\t'; p++) + for (; p < end && *p != '\0' && *p != ' ' && *p != '\t'; p++) ; /* and remove trailing whitespace because base64_decode needs this */ - *p = '\0'; - len = base64_decode(encoded, target, targsize); - free(encoded); - return len; + return base64_decode(src, p - src, result, decoded_size); } diff --git a/base64.h b/base64.h index a06bf2ec..4bfaa99d 100644 --- a/base64.h +++ b/base64.h @@ -1,2 +1,4 @@ -int uudecode(const char *src, unsigned char *target, size_t targsize); -int base64_decode(char const *src, unsigned char *target, size_t targsize); +int uudecode(char const *src, size_t encoded_size, char **result, + size_t *decoded_size); +int base64_decode(char const *src, size_t encoded_size, char **result, + size_t *decoded_size); diff --git a/crypt.c b/crypt.c index 222bece7..f227eb39 100644 --- a/crypt.c +++ b/crypt.c @@ -159,7 +159,7 @@ int get_asymmetric_key(const char *key_file, int private, struct asymmetric_key *key = NULL; void *map = NULL; unsigned char *blob = NULL; - size_t map_size, blob_size, decoded_size; + size_t map_size, encoded_size, decoded_size; int ret, ret2; char *cp; @@ -181,16 +181,11 @@ int get_asymmetric_key(const char *key_file, int private, goto out; } cp = map + ret; + encoded_size = map_size - ret; PARA_INFO_LOG("decoding public rsa-ssh key %s\n", key_file); - ret = -ERRNO_TO_PARA_ERROR(EOVERFLOW); - if (map_size > INT_MAX / 4) - goto out_unmap; - blob_size = 2 * map_size; - blob = para_malloc(blob_size); - ret = uudecode(cp, blob, blob_size); + ret = uudecode(cp, encoded_size, (char **)&blob, &decoded_size); if (ret < 0) goto out_unmap; - decoded_size = ret; ret = check_ssh_key_header(blob, decoded_size); if (ret < 0) goto out_unmap; diff --git a/gcrypt.c b/gcrypt.c index 8e73b3b2..289748e8 100644 --- a/gcrypt.c +++ b/gcrypt.c @@ -240,12 +240,11 @@ static int decode_key(const char *key_file, const char *header_str, key[j++] = begin[i]; } key[j] = '\0'; - blob_size = key_size * 2; - blob = para_malloc(blob_size); - ret = base64_decode(key, blob, blob_size); + ret = base64_decode(key, j, (char **)&blob, &blob_size); free(key); if (ret < 0) goto free_unmap; + ret = blob_size; goto unmap; free_unmap: free(blob); @@ -607,13 +606,9 @@ static int get_ssh_public_key(unsigned char *data, int size, gcry_sexp_t *result gcry_mpi_t e = NULL, n = NULL; PARA_DEBUG_LOG("decoding %d byte public rsa-ssh key\n", size); - if (size > INT_MAX / 4) - return -ERRNO_TO_PARA_ERROR(EOVERFLOW); - blob = para_malloc(2 * size); - ret = uudecode((char *)data, blob, 2 * size); + ret = uudecode((char *)data, size, (char **)&blob, &decoded_size); if (ret < 0) goto free_blob; - decoded_size = ret; end = blob + decoded_size; dump_buffer("decoded key", blob, decoded_size); ret = check_ssh_key_header(blob, decoded_size);