[PATCH 3/5] crypt32: Fix base64 issues in CryptStringToBinary.

Lauri Kenttä lauri.kentta at gmail.com
Sun Feb 12 05:47:52 CST 2017


The old base64 decoder design is ill-suited for handling non-typical
base64, so this patch replaces it completely. Also, the old decoder
fails to honor the provided buffer length, which is also fixed here.

Signed-off-by: Lauri Kenttä <lauri.kentta at gmail.com>
---
 dlls/crypt32/base64.c       | 280 ++++++++++++++------------------------------
 dlls/crypt32/tests/base64.c |   9 +-
 2 files changed, 90 insertions(+), 199 deletions(-)

diff --git a/dlls/crypt32/base64.c b/dlls/crypt32/base64.c
index 9b93bc6433..9084789b12 100644
--- a/dlls/crypt32/base64.c
+++ b/dlls/crypt32/base64.c
@@ -483,9 +483,13 @@ BOOL WINAPI CryptBinaryToStringW(const BYTE *pbBinary,
     return encoder(pbBinary, cbBinary, dwFlags, pszString, pcchString);
 }
 
-static inline BYTE decodeBase64Byte(int c)
+#define BASE64_DECODE_PADDING    0x100
+#define BASE64_DECODE_WHITESPACE 0x200
+#define BASE64_DECODE_INVALID    0x300
+
+static inline int decodeBase64Byte(int c)
 {
-    BYTE ret;
+    int ret = BASE64_DECODE_INVALID;
 
     if (c >= 'A' && c <= 'Z')
         ret = c - 'A';
@@ -497,117 +501,105 @@ static inline BYTE decodeBase64Byte(int c)
         ret = 62;
     else if (c == '/')
         ret = 63;
-    else
-        ret = 64;
+    else if (c == '=')
+        ret = BASE64_DECODE_PADDING;
+    else if (c == ' ' || c == '\t' || c == '\r' || c == '\n')
+        ret = BASE64_DECODE_WHITESPACE;
     return ret;
 }
 
-static LONG decodeBase64Block(const char *in_buf, int in_len,
- const char **nextBlock, PBYTE out_buf, DWORD *out_len)
-{
-    int len = in_len;
-    const char *d = in_buf;
-    int  ip0, ip1, ip2, ip3;
-
-    if (len < 4)
-        return ERROR_INVALID_DATA;
+/* Unlike CryptStringToBinaryA, cchString is guaranteed to be the length of the
+ * string to convert.
+ */
+typedef LONG (*StringToBinaryAFunc)(LPCSTR pszString, DWORD cchString,
+ BYTE *pbBinary, DWORD *pcbBinary, DWORD *pdwSkip, DWORD *pdwFlags);
 
-    if (d[2] == '=')
+static LONG Base64ToBinary(const void* pszString, BOOL wide, DWORD cchString,
+ BYTE *pbBinary, DWORD *pcbBinary, DWORD *pdwSkip, DWORD *pdwFlags)
+{
+    DWORD cbIn, cbValid, cbOut, hasPadding;
+    BYTE block[4];
+    for (cbIn = cbValid = cbOut = hasPadding = 0; cbIn < cchString; ++cbIn)
     {
-        if ((ip0 = decodeBase64Byte(d[0])) > 63)
-            return ERROR_INVALID_DATA;
-        if ((ip1 = decodeBase64Byte(d[1])) > 63)
-            return ERROR_INVALID_DATA;
+        int c = wide ? (int)((WCHAR*)pszString)[cbIn] : (int)((char*)pszString)[cbIn];
+        int d = decodeBase64Byte(c);
+        if (d == BASE64_DECODE_INVALID)
+            goto invalid;
+        if (d == BASE64_DECODE_WHITESPACE)
+            continue;
+
+        /* When padding starts, data is not acceptable */
+        if (hasPadding && d != BASE64_DECODE_PADDING)
+            goto invalid;
+
+        /* Padding after a full block (like "VVVV=") is ok and stops decoding */
+        if (d == BASE64_DECODE_PADDING && (cbValid & 3) == 0)
+            break;
 
-        if (out_buf)
-            out_buf[0] = (ip0 << 2) | (ip1 >> 4);
-        *out_len = 1;
-    }
-    else if (d[3] == '=')
-    {
-        if ((ip0 = decodeBase64Byte(d[0])) > 63)
-            return ERROR_INVALID_DATA;
-        if ((ip1 = decodeBase64Byte(d[1])) > 63)
-            return ERROR_INVALID_DATA;
-        if ((ip2 = decodeBase64Byte(d[2])) > 63)
-            return ERROR_INVALID_DATA;
+        cbValid += 1;
 
-        if (out_buf)
+        if (d == BASE64_DECODE_PADDING)
         {
-            out_buf[0] = (ip0 << 2) | (ip1 >> 4);
-            out_buf[1] = (ip1 << 4) | (ip2 >> 2);
+            hasPadding = 1;
+            /* When padding reaches a full block, stop decoding */
+            if ((cbValid & 3) == 0)
+                break;
+            continue;
         }
-        *out_len = 2;
+
+        /* cbOut is incremented in the 4-char block as follows: "1-23" */
+        if ((cbValid & 3) != 2)
+            cbOut += 1;
     }
-    else
+    /* Fail if the block has bad padding; omitting padding is fine */
+    if ((cbValid & 3) != 0 && hasPadding)
+        goto invalid;
+    /* Check available buffer size */
+    if (pbBinary && *pcbBinary && cbOut > *pcbBinary)
+        goto overflow;
+    /* Convert the data; this step depends on the validity checks above! */
+    if (pbBinary) for (cbIn = cbValid = cbOut = 0; cbIn < cchString; ++cbIn)
     {
-        if ((ip0 = decodeBase64Byte(d[0])) > 63)
-            return ERROR_INVALID_DATA;
-        if ((ip1 = decodeBase64Byte(d[1])) > 63)
-            return ERROR_INVALID_DATA;
-        if ((ip2 = decodeBase64Byte(d[2])) > 63)
-            return ERROR_INVALID_DATA;
-        if ((ip3 = decodeBase64Byte(d[3])) > 63)
-            return ERROR_INVALID_DATA;
-
-        if (out_buf)
-        {
-            out_buf[0] = (ip0 << 2) | (ip1 >> 4);
-            out_buf[1] = (ip1 << 4) | (ip2 >> 2);
-            out_buf[2] = (ip2 << 6) |  ip3;
+        int c = wide ? (int)((WCHAR*)pszString)[cbIn] : (int)((char*)pszString)[cbIn];
+        int d = decodeBase64Byte(c);
+        if (d == BASE64_DECODE_WHITESPACE)
+            continue;
+        if (d == BASE64_DECODE_PADDING)
+            break;
+        block[cbValid & 3] = d;
+        cbValid += 1;
+        switch (cbValid & 3) {
+        case 1:
+            pbBinary[cbOut++] = (block[0] << 2);
+            break;
+        case 2:
+            pbBinary[cbOut-1] = (block[0] << 2) | (block[1] >> 4);
+            break;
+        case 3:
+            pbBinary[cbOut++] = (block[1] << 4) | (block[2] >> 2);
+            break;
+        case 0:
+            pbBinary[cbOut++] = (block[2] << 6) | (block[3] >> 0);
+            break;
         }
-        *out_len = 3;
     }
-    if (len >= 6 && d[4] == '\r' && d[5] == '\n')
-        *nextBlock = d + 6;
-    else if (len >= 5 && d[4] == '\n')
-        *nextBlock = d + 5;
-    else if (len >= 4 && d[4])
-        *nextBlock = d + 4;
-    else
-        *nextBlock = NULL;
+    *pcbBinary = cbOut;
+    if (pdwSkip)
+        *pdwSkip = 0;
+    if (pdwFlags)
+        *pdwFlags = CRYPT_STRING_BASE64;
     return ERROR_SUCCESS;
+overflow:
+    return ERROR_INSUFFICIENT_BUFFER;
+invalid:
+    *pcbBinary = cbOut;
+    return ERROR_INVALID_DATA;
 }
 
-/* Unlike CryptStringToBinaryA, cchString is guaranteed to be the length of the
- * string to convert.
- */
-typedef LONG (*StringToBinaryAFunc)(LPCSTR pszString, DWORD cchString,
- BYTE *pbBinary, DWORD *pcbBinary, DWORD *pdwSkip, DWORD *pdwFlags);
-
 static LONG Base64ToBinaryA(LPCSTR pszString, DWORD cchString,
  BYTE *pbBinary, DWORD *pcbBinary, DWORD *pdwSkip, DWORD *pdwFlags)
 {
-    LONG ret = ERROR_SUCCESS;
-    const char *nextBlock;
-    DWORD outLen = 0;
-
-    nextBlock = pszString;
-    while (nextBlock && !ret)
-    {
-        DWORD len = 0;
-
-        ret = decodeBase64Block(nextBlock, cchString - (nextBlock - pszString),
-         &nextBlock, pbBinary ? pbBinary + outLen : NULL, &len);
-        if (!ret)
-            outLen += len;
-        if (cchString - (nextBlock - pszString) <= 0)
-            nextBlock = NULL;
-    }
-    *pcbBinary = outLen;
-    if (!ret)
-    {
-        if (pdwSkip)
-            *pdwSkip = 0;
-        if (pdwFlags)
-            *pdwFlags = CRYPT_STRING_BASE64;
-    }
-    else if (ret == ERROR_INSUFFICIENT_BUFFER)
-    {
-        if (!pbBinary)
-            ret = ERROR_SUCCESS;
-    }
-    return ret;
+    return Base64ToBinary(pszString, FALSE, cchString, pbBinary, pcbBinary, pdwSkip, pdwFlags);
 }
 
 static LONG Base64WithHeaderAndTrailerToBinaryA(LPCSTR pszString,
@@ -830,75 +822,6 @@ BOOL WINAPI CryptStringToBinaryA(LPCSTR pszString,
     return ret == ERROR_SUCCESS;
 }
 
-static LONG decodeBase64BlockW(const WCHAR *in_buf, int in_len,
- const WCHAR **nextBlock, PBYTE out_buf, DWORD *out_len)
-{
-    int len = in_len, i;
-    const WCHAR *d = in_buf;
-    int  ip0, ip1, ip2, ip3;
-
-    if (len < 4)
-        return ERROR_INVALID_DATA;
-
-    i = 0;
-    if (d[2] == '=')
-    {
-        if ((ip0 = decodeBase64Byte(d[0])) > 63)
-            return ERROR_INVALID_DATA;
-        if ((ip1 = decodeBase64Byte(d[1])) > 63)
-            return ERROR_INVALID_DATA;
-
-        if (out_buf)
-            out_buf[i] = (ip0 << 2) | (ip1 >> 4);
-        i++;
-    }
-    else if (d[3] == '=')
-    {
-        if ((ip0 = decodeBase64Byte(d[0])) > 63)
-            return ERROR_INVALID_DATA;
-        if ((ip1 = decodeBase64Byte(d[1])) > 63)
-            return ERROR_INVALID_DATA;
-        if ((ip2 = decodeBase64Byte(d[2])) > 63)
-            return ERROR_INVALID_DATA;
-
-        if (out_buf)
-        {
-            out_buf[i + 0] = (ip0 << 2) | (ip1 >> 4);
-            out_buf[i + 1] = (ip1 << 4) | (ip2 >> 2);
-        }
-        i += 2;
-    }
-    else
-    {
-        if ((ip0 = decodeBase64Byte(d[0])) > 63)
-            return ERROR_INVALID_DATA;
-        if ((ip1 = decodeBase64Byte(d[1])) > 63)
-            return ERROR_INVALID_DATA;
-        if ((ip2 = decodeBase64Byte(d[2])) > 63)
-            return ERROR_INVALID_DATA;
-        if ((ip3 = decodeBase64Byte(d[3])) > 63)
-            return ERROR_INVALID_DATA;
-
-        if (out_buf)
-        {
-            out_buf[i + 0] = (ip0 << 2) | (ip1 >> 4);
-            out_buf[i + 1] = (ip1 << 4) | (ip2 >> 2);
-            out_buf[i + 2] = (ip2 << 6) |  ip3;
-        }
-        i += 3;
-    }
-    if (len >= 6 && d[4] == '\r' && d[5] == '\n')
-        *nextBlock = d + 6;
-    else if (len >= 5 && d[4] == '\n')
-        *nextBlock = d + 5;
-    else if (len >= 4 && d[4])
-        *nextBlock = d + 4;
-    else
-        *nextBlock = NULL;
-    *out_len = i;
-    return ERROR_SUCCESS;
-}
-
 /* Unlike CryptStringToBinaryW, cchString is guaranteed to be the length of the
  * string to convert.
  */
@@ -908,36 +831,7 @@ typedef LONG (*StringToBinaryWFunc)(LPCWSTR pszString, DWORD cchString,
 static LONG Base64ToBinaryW(LPCWSTR pszString, DWORD cchString,
  BYTE *pbBinary, DWORD *pcbBinary, DWORD *pdwSkip, DWORD *pdwFlags)
 {
-    LONG ret = ERROR_SUCCESS;
-    const WCHAR *nextBlock;
-    DWORD outLen = 0;
-
-    nextBlock = pszString;
-    while (nextBlock && !ret)
-    {
-        DWORD len = 0;
-
-        ret = decodeBase64BlockW(nextBlock, cchString - (nextBlock - pszString),
-         &nextBlock, pbBinary ? pbBinary + outLen : NULL, &len);
-        if (!ret)
-            outLen += len;
-        if (cchString - (nextBlock - pszString) <= 0)
-            nextBlock = NULL;
-    }
-    *pcbBinary = outLen;
-    if (!ret)
-    {
-        if (pdwSkip)
-            *pdwSkip = 0;
-        if (pdwFlags)
-            *pdwFlags = CRYPT_STRING_BASE64;
-    }
-    else if (ret == ERROR_INSUFFICIENT_BUFFER)
-    {
-        if (!pbBinary)
-            ret = ERROR_SUCCESS;
-    }
-    return ret;
+    return Base64ToBinary(pszString, TRUE, cchString, pbBinary, pcbBinary, pdwSkip, pdwFlags);
 }
 
 static LONG Base64WithHeaderAndTrailerToBinaryW(LPCWSTR pszString,
diff --git a/dlls/crypt32/tests/base64.c b/dlls/crypt32/tests/base64.c
index 82668113c2..f28c051f56 100644
--- a/dlls/crypt32/tests/base64.c
+++ b/dlls/crypt32/tests/base64.c
@@ -385,7 +385,6 @@ static void testStringToBinaryA(void)
     decodeBase64WithLen("V=", 2, 0, ERROR_INVALID_DATA);
     decodeBase64WithLen("VV=", 3, 0, ERROR_INVALID_DATA);
     decodeBase64WithLen("V==", 3, 0, ERROR_INVALID_DATA);
-    todo_wine {
     decodeBase64WithLenBroken("V", 0, "T", 0);
     decodeBase64WithLenBroken("VV", 0, "U", 0);
     decodeBase64WithLenBroken("VVV", 0, "UU", 0);
@@ -410,18 +409,17 @@ static void testStringToBinaryA(void)
     decodeBase64WithLen("VV==VVVV", 8, "U", 0);
     decodeBase64WithLen("VVV=VVVV", 8, "UU", 0);
     decodeBase64WithLen("VVVV=VVVV", 8, "UUU", 0);
-    }
 
     decodeBase64WithFmt("-----BEGIN-----VVVV-----END-----", CRYPT_STRING_BASE64HEADER, 0, ERROR_INVALID_DATA);
     decodeBase64WithFmt("-----BEGIN-----VVVV-----END -----", CRYPT_STRING_BASE64HEADER, 0, ERROR_INVALID_DATA);
     decodeBase64WithFmt("-----BEGIN -----VVVV-----END-----", CRYPT_STRING_BASE64HEADER, 0, ERROR_INVALID_DATA);
     decodeBase64WithFmt("-----BEGIN -----VVVV-----END -----", CRYPT_STRING_BASE64HEADER, "UUU", 0);
 
-    todo_wine {
     decodeBase64WithFmt("-----BEGIN -----V-----END -----", CRYPT_STRING_BASE64HEADER, "T", 0);
     decodeBase64WithFmt("-----BEGIN foo-----V-----END -----", CRYPT_STRING_BASE64HEADER, "T", 0);
     decodeBase64WithFmt("-----BEGIN foo-----V-----END foo-----", CRYPT_STRING_BASE64HEADER, "T", 0);
     decodeBase64WithFmt("-----BEGIN -----V-----END foo-----", CRYPT_STRING_BASE64HEADER, "T", 0);
+    todo_wine {
     decodeBase64WithFmt("-----BEGIN -----V-----END -----", CRYPT_STRING_BASE64X509CRLHEADER, "T", 0);
     decodeBase64WithFmt("-----BEGIN foo-----V-----END -----", CRYPT_STRING_BASE64X509CRLHEADER, "T", 0);
     decodeBase64WithFmt("-----BEGIN foo-----V-----END foo-----", CRYPT_STRING_BASE64X509CRLHEADER, "T", 0);
@@ -436,7 +434,6 @@ static void testStringToBinaryA(void)
     buf[0] = 0;
     bufLen = 4;
     ret = pCryptStringToBinaryA("VVVVVVVV", 8, CRYPT_STRING_BASE64, (BYTE*)buf, &bufLen, NULL, NULL);
-    todo_wine
     ok(!ret && bufLen == 4 && buf[0] == 0,
      "Expected ret 0, bufLen 4, buf[0] '\\0', got ret %d, bufLen %d, buf[0] '%c'\n",
      ret, bufLen, buf[0]);
@@ -450,7 +447,7 @@ static void testStringToBinaryA(void)
          */
         ret = pCryptStringToBinaryA(tests[i].base64, 1, CRYPT_STRING_BASE64,
          NULL, &bufLen, NULL, NULL);
-        todo_wine ok(ret, "CryptStringToBinaryA failed: %d\n", GetLastError());
+        ok(ret, "CryptStringToBinaryA failed: %d\n", GetLastError());
         /* Check with the precise format */
         decodeAndCompareBase64_A(tests[i].base64, NULL, NULL,
          CRYPT_STRING_BASE64, CRYPT_STRING_BASE64, tests[i].toEncode,
@@ -507,7 +504,7 @@ static void testStringToBinaryA(void)
          */
         ret = pCryptStringToBinaryA(testsNoCR[i].base64, 1, CRYPT_STRING_BASE64,
          NULL, &bufLen, NULL, NULL);
-        todo_wine ok(ret, "CryptStringToBinaryA failed: %d\n", GetLastError());
+        ok(ret, "CryptStringToBinaryA failed: %d\n", GetLastError());
         /* Check with the precise format */
         decodeAndCompareBase64_A(testsNoCR[i].base64, NULL, NULL,
          CRYPT_STRING_BASE64, CRYPT_STRING_BASE64, testsNoCR[i].toEncode,
-- 
2.11.1




More information about the wine-patches mailing list