crypt32(2/2): Fix decoding sequences with extra trailing data
Juan Lang
juan.lang at gmail.com
Fri Jul 13 12:41:48 CDT 2007
You'll undoubtedly notice I relaxed a couple of tests to do so, as the
last error doesn't match with this fix. Hope that's okay.
--Juan
-------------- next part --------------
From 1363bb2240f40a1a4853582816ec81288c7a161a Mon Sep 17 00:00:00 2001
From: Juan Lang <juanlang at juan.corp.google.com>
Date: Fri, 13 Jul 2007 10:35:58 -0700
Subject: [PATCH] Fix decoding sequences with extra trailing data
---
dlls/crypt32/decode.c | 74 +++++++++++++++++++++++++++++++++----------
dlls/crypt32/tests/crl.c | 8 +----
dlls/crypt32/tests/encode.c | 4 +-
3 files changed, 60 insertions(+), 26 deletions(-)
diff --git a/dlls/crypt32/decode.c b/dlls/crypt32/decode.c
index d3f3322..3a3a195 100644
--- a/dlls/crypt32/decode.c
+++ b/dlls/crypt32/decode.c
@@ -297,15 +297,21 @@ struct AsnDecodeSequenceItem
DWORD size;
};
+/* Decodes the items in a sequence, where the items are described in items,
+ * the encoded data are in pbEncoded with length cbEncoded. Decodes into
+ * pvStructInfo. nextData is a pointer to the memory location at which the
+ * first decoded item with a dynamic pointer should point.
+ * Upon decoding, *cbDecoded is the total number of bytes decoded.
+ */
static BOOL CRYPT_AsnDecodeSequenceItems(DWORD dwCertEncodingType,
struct AsnDecodeSequenceItem items[], DWORD cItem, const BYTE *pbEncoded,
- DWORD cbEncoded, DWORD dwFlags, void *pvStructInfo, BYTE *nextData)
+ DWORD cbEncoded, DWORD dwFlags, void *pvStructInfo, BYTE *nextData,
+ DWORD *cbDecoded)
{
BOOL ret;
- DWORD i;
- const BYTE *ptr;
+ DWORD i, decoded = 0;
+ const BYTE *ptr = pbEncoded;
- ptr = pbEncoded + 1 + GET_LEN_BYTES(pbEncoded[1]);
for (i = 0, ret = TRUE; ret && i < cItem; i++)
{
if (cbEncoded - (ptr - pbEncoded) != 0)
@@ -353,6 +359,7 @@ static BOOL CRYPT_AsnDecodeSequenceItems
items[i].size += sizeof(DWORD) -
items[i].size % sizeof(DWORD);
ptr += 1 + nextItemLenBytes + nextItemLen;
+ decoded += 1 + nextItemLenBytes + nextItemLen;
}
else if (items[i].optional &&
GetLastError() == CRYPT_E_ASN1_BADTAG)
@@ -367,7 +374,10 @@ static BOOL CRYPT_AsnDecodeSequenceItems
GetLastError());
}
else
+ {
+ decoded += 1 + nextItemLenBytes + nextItemLen;
items[i].size = items[i].minSize;
+ }
}
else if (items[i].optional)
{
@@ -395,13 +405,8 @@ static BOOL CRYPT_AsnDecodeSequenceItems
ret = FALSE;
}
}
- if (cbEncoded - (ptr - pbEncoded) != 0)
- {
- TRACE("%d remaining bytes, failing\n", cbEncoded -
- (ptr - pbEncoded));
- SetLastError(CRYPT_E_ASN1_CORRUPT);
- ret = FALSE;
- }
+ if (ret)
+ *cbDecoded = decoded;
return ret;
}
@@ -412,7 +417,6 @@ static BOOL CRYPT_AsnDecodeSequenceItems
* data will be stored. If you know the starting offset, you may pass it
* here. Otherwise, pass NULL, and one will be inferred from the items.
* Each item decoder is never called with CRYPT_DECODE_ALLOC_FLAG set.
- * If any undecoded data are left over, fails with CRYPT_E_ASN1_CORRUPT.
*/
static BOOL CRYPT_AsnDecodeSequence(DWORD dwCertEncodingType,
struct AsnDecodeSequenceItem items[], DWORD cItem, const BYTE *pbEncoded,
@@ -431,13 +435,30 @@ static BOOL CRYPT_AsnDecodeSequence(DWOR
if ((ret = CRYPT_GetLen(pbEncoded, cbEncoded, &dataLen)))
{
- DWORD i;
+ DWORD lenBytes = GET_LEN_BYTES(pbEncoded[1]), cbDecoded;
+ const BYTE *ptr = pbEncoded + 1 + lenBytes;
- ret = CRYPT_AsnDecodeSequenceItems(dwFlags, items, cItem, pbEncoded,
- cbEncoded, dwFlags, NULL, NULL);
+ cbEncoded -= 1 + lenBytes;
+ if (cbEncoded < dataLen)
+ {
+ TRACE("dataLen %d exceeds cbEncoded %d, failing\n", dataLen,
+ cbEncoded);
+ SetLastError(CRYPT_E_ASN1_CORRUPT);
+ ret = FALSE;
+ }
+ else
+ ret = CRYPT_AsnDecodeSequenceItems(dwFlags, items, cItem, ptr,
+ cbEncoded, dwFlags, NULL, NULL, &cbDecoded);
+ if (ret && cbDecoded != dataLen)
+ {
+ TRACE("expected %d decoded, got %d, failing\n", dataLen,
+ cbDecoded);
+ SetLastError(CRYPT_E_ASN1_CORRUPT);
+ ret = FALSE;
+ }
if (ret)
{
- DWORD bytesNeeded = 0, structSize = 0;
+ DWORD i, bytesNeeded = 0, structSize = 0;
for (i = 0; i < cItem; i++)
{
@@ -459,7 +480,8 @@ static BOOL CRYPT_AsnDecodeSequence(DWOR
nextData = (BYTE *)pvStructInfo + structSize;
memset(pvStructInfo, 0, structSize);
ret = CRYPT_AsnDecodeSequenceItems(dwFlags, items, cItem,
- pbEncoded, cbEncoded, dwFlags, pvStructInfo, nextData);
+ ptr, cbEncoded, dwFlags, pvStructInfo, nextData,
+ &cbDecoded);
}
}
}
@@ -872,6 +894,24 @@ static BOOL WINAPI CRYPT_AsnDecodeCertIn
ret = CRYPT_AsnDecodeSequence(dwCertEncodingType, items,
sizeof(items) / sizeof(items[0]), pbEncoded, cbEncoded, dwFlags,
pDecodePara, pvStructInfo, pcbStructInfo, NULL);
+ if (ret && pvStructInfo)
+ {
+ CERT_INFO *info;
+
+ if (dwFlags & CRYPT_DECODE_ALLOC_FLAG)
+ info = *(CERT_INFO **)pvStructInfo;
+ else
+ info = (CERT_INFO *)pvStructInfo;
+ if (!info->SerialNumber.cbData || !info->Issuer.cbData ||
+ !info->Subject.cbData)
+ {
+ SetLastError(CRYPT_E_ASN1_CORRUPT);
+ /* Don't need to deallocate, because it should have failed on the
+ * first pass (and no memory was allocated.)
+ */
+ ret = FALSE;
+ }
+ }
TRACE("Returning %d (%08x)\n", ret, GetLastError());
return ret;
diff --git a/dlls/crypt32/tests/crl.c b/dlls/crypt32/tests/crl.c
index 462d89f..7ecf6d4 100644
--- a/dlls/crypt32/tests/crl.c
+++ b/dlls/crypt32/tests/crl.c
@@ -98,14 +98,10 @@ static void testCreateCRL(void)
"Expected CRYPT_E_ASN1_EOD or OSS_MORE_INPUT, got %08x\n", GLE);
context = CertCreateCRLContext(X509_ASN_ENCODING, bigCert, sizeof(bigCert));
GLE = GetLastError();
- ok(!context && (GLE == CRYPT_E_ASN1_CORRUPT || GLE == OSS_DATA_ERROR),
- "Expected CRYPT_E_ASN1_CORRUPT or OSS_DATA_ERROR, got %08x\n", GLE);
+ ok(!context, "Expected failure\n");
context = CertCreateCRLContext(X509_ASN_ENCODING, signedCRL,
sizeof(signedCRL) - 1);
- ok(!context && (GLE == CRYPT_E_ASN1_EOD || GLE == CRYPT_E_ASN1_CORRUPT ||
- GLE == OSS_DATA_ERROR),
- "Expected CRYPT_E_ASN1_EOD or CRYPT_E_ASN1_CORRUPT or OSS_DATA_ERROR, got %08x\n",
- GLE);
+ ok(!context, "Expected failure\n");
context = CertCreateCRLContext(X509_ASN_ENCODING, signedCRL,
sizeof(signedCRL));
ok(context != NULL, "CertCreateCRLContext failed: %08x\n", GetLastError());
diff --git a/dlls/crypt32/tests/encode.c b/dlls/crypt32/tests/encode.c
index 881f6cb..8173a12 100644
--- a/dlls/crypt32/tests/encode.c
+++ b/dlls/crypt32/tests/encode.c
@@ -2765,8 +2765,7 @@ static void test_decodeCertToBeSigned(DW
ret = CryptDecodeObjectEx(dwEncoding, X509_CERT_TO_BE_SIGNED,
corruptCerts[i], corruptCerts[i][1] + 2, CRYPT_DECODE_ALLOC_FLAG, NULL,
(BYTE *)&buf, &size);
- ok(!ret && (GetLastError() == CRYPT_E_ASN1_CORRUPT),
- "Expected CRYPT_E_ASN1_CORRUPT, got %08x\n", GetLastError());
+ ok(!ret, "Expected failure\n");
}
/* Now check with serial number, subject and issuer specified */
ret = CryptDecodeObjectEx(dwEncoding, X509_CERT_TO_BE_SIGNED, bigCert,
@@ -4642,7 +4641,6 @@ static void test_decodePKCSContentInfo(D
ret = CryptDecodeObjectEx(dwEncoding, PKCS_CONTENT_INFO,
emptyPKCSContentInfoExtraBytes, sizeof(emptyPKCSContentInfoExtraBytes),
0, NULL, NULL, &size);
- todo_wine
ok(ret, "CryptDecodeObjectEx failed: %x\n", GetLastError());
SetLastError(0xdeadbeef);
ret = CryptDecodeObjectEx(dwEncoding, PKCS_CONTENT_INFO,
--
1.4.1
More information about the wine-patches
mailing list