From 820d6ea6a43590a9bbd9b908154f2632c290107c Mon Sep 17 00:00:00 2001 From: Kevin Jacobs Date: Thu, 24 Oct 2019 16:47:28 +0200 Subject: Add length checks for cryptographic primitives This rollup patch adds additional length checks around cryptographic primitives. --- security/nss/lib/freebl/chacha20poly1305.c | 5 +++++ security/nss/lib/freebl/ctr.c | 12 ++++++++++++ security/nss/lib/freebl/gcm.c | 6 ++++++ security/nss/lib/freebl/intel-gcm-wrap.c | 22 ++++++++++++++++++++++ security/nss/lib/freebl/rsapkcs.c | 20 +++++++++++--------- 5 files changed, 56 insertions(+), 9 deletions(-) diff --git a/security/nss/lib/freebl/chacha20poly1305.c b/security/nss/lib/freebl/chacha20poly1305.c index 302f0db9e..8fdaf3fec 100644 --- a/security/nss/lib/freebl/chacha20poly1305.c +++ b/security/nss/lib/freebl/chacha20poly1305.c @@ -234,6 +234,11 @@ ChaCha20Poly1305_Open(const ChaCha20Poly1305Context *ctx, unsigned char *output, PORT_SetError(SEC_ERROR_OUTPUT_LEN); return SECFailure; } + // ChaCha has a 64 octet block, with a 32-bit block counter. + if (inputLen >= (1ULL << (6 + 32)) + ctx->tagLen) { + PORT_SetError(SEC_ERROR_INPUT_LEN); + return SECFailure; + } PORT_Memset(block, 0, sizeof(block)); // Generate a block of keystream. The first 32 bytes will be the poly1305 diff --git a/security/nss/lib/freebl/ctr.c b/security/nss/lib/freebl/ctr.c index d7652c060..4d26a5b06 100644 --- a/security/nss/lib/freebl/ctr.c +++ b/security/nss/lib/freebl/ctr.c @@ -128,6 +128,12 @@ CTR_Update(CTRContext *ctr, unsigned char *outbuf, unsigned int tmp; SECStatus rv; + // Limit block count to 2^counterBits - 2 + if (ctr->counterBits < (sizeof(unsigned int) * 8) && + inlen > ((1 << ctr->counterBits) - 2) * AES_BLOCK_SIZE) { + PORT_SetError(SEC_ERROR_INPUT_LEN); + return SECFailure; + } if (maxout < inlen) { *outlen = inlen; PORT_SetError(SEC_ERROR_OUTPUT_LEN); @@ -199,6 +205,12 @@ CTR_Update_HW_AES(CTRContext *ctr, unsigned char *outbuf, unsigned int tmp; SECStatus rv; + // Limit block count to 2^counterBits - 2 + if (ctr->counterBits < (sizeof(unsigned int) * 8) && + inlen > ((1 << ctr->counterBits) - 2) * AES_BLOCK_SIZE) { + PORT_SetError(SEC_ERROR_INPUT_LEN); + return SECFailure; + } if (maxout < inlen) { *outlen = inlen; PORT_SetError(SEC_ERROR_OUTPUT_LEN); diff --git a/security/nss/lib/freebl/gcm.c b/security/nss/lib/freebl/gcm.c index f1e16da78..e93970b88 100644 --- a/security/nss/lib/freebl/gcm.c +++ b/security/nss/lib/freebl/gcm.c @@ -469,6 +469,12 @@ gcmHash_Reset(gcmHashContext *ghash, const unsigned char *AAD, { SECStatus rv; + // Limit AADLen in accordance with SP800-38D + if (sizeof(AADLen) >= 8 && AADLen > (1ULL << 61) - 1) { + PORT_SetError(SEC_ERROR_INPUT_LEN); + return SECFailure; + } + ghash->cLen = 0; PORT_Memset(ghash->counterBuf, 0, GCM_HASH_LEN_LEN * 2); ghash->bufLen = 0; diff --git a/security/nss/lib/freebl/intel-gcm-wrap.c b/security/nss/lib/freebl/intel-gcm-wrap.c index 37a1af765..f69bc7c7a 100644 --- a/security/nss/lib/freebl/intel-gcm-wrap.c +++ b/security/nss/lib/freebl/intel-gcm-wrap.c @@ -62,6 +62,12 @@ intel_AES_GCM_CreateContext(void *context, PORT_SetError(SEC_ERROR_INVALID_ARGS); return NULL; } + // Limit AADLen in accordance with SP800-38D + if (sizeof(AAD_whole_len) >= 8 && AAD_whole_len > (1ULL << 61) - 1) { + PORT_SetError(SEC_ERROR_INPUT_LEN); + return NULL; + } + gcm = PORT_ZNew(intel_AES_GCMContext); if (gcm == NULL) { return NULL; @@ -159,6 +165,14 @@ intel_AES_GCM_EncryptUpdate(intel_AES_GCMContext *gcm, unsigned char T[AES_BLOCK_SIZE]; unsigned int j; + // GCM has a 16 octet block, with a 32-bit block counter + // Limit in accordance with SP800-38D + if (sizeof(inlen) > 4 && + inlen >= ((1ULL << 32) - 2) * AES_BLOCK_SIZE) { + PORT_SetError(SEC_ERROR_INPUT_LEN); + return SECFailure; + } + tagBytes = (gcm->tagBits + (PR_BITS_PER_BYTE - 1)) / PR_BITS_PER_BYTE; if (UINT_MAX - inlen < tagBytes) { PORT_SetError(SEC_ERROR_INPUT_LEN); @@ -216,6 +230,14 @@ intel_AES_GCM_DecryptUpdate(intel_AES_GCMContext *gcm, inlen -= tagBytes; intag = inbuf + inlen; + // GCM has a 16 octet block, with a 32-bit block counter + // Limit in accordance with SP800-38D + if (sizeof(inlen) > 4 && + inlen >= ((1ULL << 32) - 2) * AES_BLOCK_SIZE) { + PORT_SetError(SEC_ERROR_INPUT_LEN); + return SECFailure; + } + if (maxout < inlen) { *outlen = inlen; PORT_SetError(SEC_ERROR_OUTPUT_LEN); diff --git a/security/nss/lib/freebl/rsapkcs.c b/security/nss/lib/freebl/rsapkcs.c index 875e4e28d..6f94770ad 100644 --- a/security/nss/lib/freebl/rsapkcs.c +++ b/security/nss/lib/freebl/rsapkcs.c @@ -115,7 +115,7 @@ rsa_FormatOneBlock(unsigned modulusLen, { unsigned char *block; unsigned char *bp; - int padLen; + unsigned int padLen; int i, j; SECStatus rv; @@ -135,14 +135,14 @@ rsa_FormatOneBlock(unsigned modulusLen, switch (blockType) { /* - * Blocks intended for private-key operation. - */ + * Blocks intended for private-key operation. + */ case RSA_BlockPrivate: /* preferred method */ /* - * 0x00 || BT || Pad || 0x00 || ActualData - * 1 1 padLen 1 data->len - * Pad is either all 0x00 or all 0xff bytes, depending on blockType. - */ + * 0x00 || BT || Pad || 0x00 || ActualData + * 1 1 padLen 1 data->len + * Pad is either all 0x00 or all 0xff bytes, depending on blockType. + */ padLen = modulusLen - data->len - 3; PORT_Assert(padLen >= RSA_BLOCK_MIN_PAD_LEN); if (padLen < RSA_BLOCK_MIN_PAD_LEN) { @@ -162,7 +162,7 @@ rsa_FormatOneBlock(unsigned modulusLen, /* * 0x00 || BT || Pad || 0x00 || ActualData * 1 1 padLen 1 data->len - * Pad is all non-zero random bytes. + * Pad is 8 or more non-zero random bytes. * * Build the block left to right. * Fill the entire block from Pad to the end with random bytes. @@ -236,7 +236,9 @@ rsa_FormatBlock(SECItem *result, * The "3" below is the first octet + the second octet + the 0x00 * octet that always comes just before the ActualData. */ - PORT_Assert(data->len <= (modulusLen - (3 + RSA_BLOCK_MIN_PAD_LEN))); + if (data->len > (modulusLen - (3 + RSA_BLOCK_MIN_PAD_LEN))) { + return SECFailure; + } result->data = rsa_FormatOneBlock(modulusLen, blockType, data); if (result->data == NULL) { -- cgit v1.2.3