base64: Saner semantics for base64_decode() and uudecode().
authorAndre Noll <maan@tuebingen.mpg.de>
Sun, 26 Apr 2015 21:41:00 +0000 (23:41 +0200)
committerAndre Noll <maan@tuebingen.mpg.de>
Tue, 23 Aug 2016 14:32:12 +0000 (16:32 +0200)
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.

base64.c
base64.h
crypt.c
gcrypt.c

index e989224..4809a2f 100644 (file)
--- 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);
 }
index a06bf2e..4bfaa99 100644 (file)
--- 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 222bece..f227eb3 100644 (file)
--- 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;
index 8e73b3b..289748e 100644 (file)
--- 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);