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 e9892244d62a0e00a320003bb3fe740e5e89f866..4809a2f285f63dd83132f693be23f68945fce21d 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 a06bf2ec160e52ebb93d48a0cf86b3f19bf1a3e8..4bfaa99d05eb6d3631d978f1fac2827823f8ec57 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 222bece7249add6aa01639419d63a42426ca9c84..f227eb39907691d24f0eae360c04027c1653be75 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 8e73b3b2696f0aee501c6a317280067b885e3011..289748e84d16dbca133b9280dc2027e5d9c2ec67 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);