[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