yuzu-emu/mbedtls
2
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity

pkcs7: Ensure all data in asn1 structure is accounted for

Browse source 
Several PKCS7 invalid ASN1 Tests were failing due to extra
data bytes or incorrect content lengths going unnoticed. Make
the parser aware of possible malformed ASN1 data.

Signed-off-by: Nick Child <nick.child@ibm.com>
This commit is contained in:
Nick Child 2022-12-15 15:54:03 -06:00
parent 4983ddf747
commit ec81709516
3 changed files with 38 additions and 11 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

5
include/mbedtls/pkcs7.h
View file

@ -179,8 +179,9 @@ void mbedtls_pkcs7_init(mbedtls_pkcs7 *pkcs7);
* \brief Parse a single DER formatted pkcs7 content.
*
* \param pkcs7 The pkcs7 structure to be filled by parser for the output.
* \param buf The buffer holding the DER encoded pkcs7.
* \param buflen The size in bytes of \p buf.
* \param buf The buffer holding only the DER encoded pkcs7.
* \param buflen The size in bytes of \p buf. The size must be exactly the
* length of the DER encoded pkcs7.
*
* \note This function makes an internal copy of the PKCS7 buffer
* \p buf. In particular, \p buf may be destroyed or reused

40
library/pkcs7.c
View file

@ -94,6 +94,7 @@ static int pkcs7_get_version(unsigned char **p, unsigned char *end, int *ver)
* [0] EXPLICIT ANY DEFINED BY contentType OPTIONAL }
**/
static int pkcs7_get_content_info_type(unsigned char **p, unsigned char *end,
unsigned char **seq_end,
mbedtls_pkcs7_buf *pkcs7)
{
size_t len = 0;
@ -106,8 +107,8 @@ static int pkcs7_get_content_info_type(unsigned char **p, unsigned char *end,
*p = start;
return MBEDTLS_ERROR_ADD(MBEDTLS_ERR_PKCS7_INVALID_CONTENT_INFO, ret);
}
ret = mbedtls_asn1_get_tag(p, end, &len, MBEDTLS_ASN1_OID);
*seq_end = *p + len;
ret = mbedtls_asn1_get_tag(p, *seq_end, &len, MBEDTLS_ASN1_OID);
if (ret != 0) {
*p = start;
return MBEDTLS_ERROR_ADD(MBEDTLS_ERR_PKCS7_INVALID_CONTENT_INFO, ret);
@ -289,7 +290,7 @@ static void pkcs7_free_signer_info(mbedtls_pkcs7_signer_info *signer)
static int pkcs7_get_signer_info(unsigned char **p, unsigned char *end,
mbedtls_pkcs7_signer_info *signer)
{
unsigned char *end_signer;
unsigned char *end_signer, *end_issuer_and_sn;
int asn1_ret = 0, ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED;
size_t len = 0;
@ -312,6 +313,7 @@ static int pkcs7_get_signer_info(unsigned char **p, unsigned char *end,
goto out;
}
end_issuer_and_sn = *p + len;
/* Parsing IssuerAndSerialNumber */
signer->issuer_raw.p = *p;
@ -333,6 +335,12 @@ static int pkcs7_get_signer_info(unsigned char **p, unsigned char *end,
goto out;
}
/* ensure no extra or missing bytes */
if (*p != end_issuer_and_sn) {
ret = MBEDTLS_ERR_PKCS7_INVALID_SIGNER_INFO;
goto out;
}
ret = pkcs7_get_digest_algorithm(p, end_signer, &signer->alg_identifier);
if (ret != 0) {
goto out;
@ -449,7 +457,7 @@ static int pkcs7_get_signed_data(unsigned char *buf, size_t buflen,
{
unsigned char *p = buf;
unsigned char *end = buf + buflen;
unsigned char *end_set;
unsigned char *end_set, *end_content_info;
size_t len = 0;
int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED;
mbedtls_md_type_t md_alg;
@ -481,11 +489,16 @@ static int pkcs7_get_signed_data(unsigned char *buf, size_t buflen,
}
/* Do not expect any content */
ret = pkcs7_get_content_info_type(&p, end_set, &signed_data->content.oid);
ret = pkcs7_get_content_info_type(&p, end_set, &end_content_info,
&signed_data->content.oid);
if (ret != 0) {
return ret;
}
if (end_content_info != p) {
return MBEDTLS_ERR_PKCS7_INVALID_CONTENT_INFO;
}
if (MBEDTLS_OID_CMP(MBEDTLS_OID_PKCS7_DATA, &signed_data->content.oid)) {
return MBEDTLS_ERR_PKCS7_INVALID_CONTENT_INFO;
}
@ -527,7 +540,7 @@ int mbedtls_pkcs7_parse_der(mbedtls_pkcs7 *pkcs7, const unsigned char *buf,
const size_t buflen)
{
unsigned char *p;
unsigned char *end;
unsigned char *end, *end_content_info;
size_t len = 0;
int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED;
int isoidset = 0;
@ -546,12 +559,19 @@ int mbedtls_pkcs7_parse_der(mbedtls_pkcs7 *pkcs7, const unsigned char *buf,
pkcs7->raw.len = buflen;
end = p + buflen;
ret = pkcs7_get_content_info_type(&p, end, &pkcs7->content_type_oid);
ret = pkcs7_get_content_info_type(&p, end, &end_content_info,
&pkcs7->content_type_oid);
if (ret != 0) {
len = buflen;
goto try_data;
}
/* Ensure PKCS7 data uses the exact number of bytes specified in buflen */
if (end_content_info != end) {
ret = MBEDTLS_ERR_PKCS7_BAD_INPUT_DATA;
goto out;
}
if (!MBEDTLS_OID_CMP(MBEDTLS_OID_PKCS7_DATA, &pkcs7->content_type_oid)
|| !MBEDTLS_OID_CMP(MBEDTLS_OID_PKCS7_ENCRYPTED_DATA, &pkcs7->content_type_oid)
|| !MBEDTLS_OID_CMP(MBEDTLS_OID_PKCS7_ENVELOPED_DATA, &pkcs7->content_type_oid)
@ -574,6 +594,12 @@ int mbedtls_pkcs7_parse_der(mbedtls_pkcs7 *pkcs7, const unsigned char *buf,
goto out;
}
/* ensure no extra/missing data */
if (p + len != end) {
ret = MBEDTLS_ERR_PKCS7_BAD_INPUT_DATA;
goto out;
}
try_data:
ret = pkcs7_get_signed_data(p, len, &pkcs7->signed_data);
if (ret != 0) {

4
tests/suites/test_suite_pkcs7.data
View file

@ -64,11 +64,11 @@ pkcs7_parse:"data_files/pkcs7_signerInfo_serial_invalid_size.der":MBEDTLS_ERR_PK
pkcs7_get_signers_info_set error handling (6213931373035520)
depends_on:MBEDTLS_RIPEMD160_C
pkcs7_parse:"data_files/pkcs7_get_signers_info_set-missing_free-fuzz_pkcs7-6213931373035520.der":MBEDTLS_ERR_PKCS7_INVALID_SIGNER_INFO + MBEDTLS_ERR_ASN1_UNEXPECTED_TAG
pkcs7_parse:"data_files/pkcs7_get_signers_info_set-missing_free-fuzz_pkcs7-6213931373035520.der":MBEDTLS_ERR_PKCS7_INVALID_CONTENT_INFO
pkcs7_get_signers_info_set error handling (4541044530479104)
depends_on:MBEDTLS_RIPEMD160_C
pkcs7_parse:"data_files/pkcs7_get_signers_info_set-leak-fuzz_pkcs7-4541044530479104.der":MBEDTLS_ERR_PKCS7_INVALID_SIGNER_INFO
pkcs7_parse:"data_files/pkcs7_get_signers_info_set-leak-fuzz_pkcs7-4541044530479104.der":MBEDTLS_ERR_PKCS7_INVALID_CONTENT_INFO
PKCS7 Only Signed Data Parse Pass #15
depends_on:MBEDTLS_SHA256_C:MBEDTLS_RSA_C