From f699f5c106ed2af844289c033de236692d7fce58 Mon Sep 17 00:00:00 2001 From: Kevin Jacobs Date: Thu, 12 Dec 2019 00:35:34 +0000 Subject: [PATCH] Bug 1603257 - Fix UBSAN issue in softoken CKM_NSS_CHACHA20_CTR initialization r=mt This patch adds an explicit cast to fix a UBSAN issue that was flagged in https://treeherder.mozilla.org/#/jobs?repo=nss-try&selectedJob=280720441. It also updates the test to use a random IV. Differential Revision: https://phabricator.services.mozilla.com/D56810 --HG-- extra : moz-landing-system : lando --- gtests/pk11_gtest/pk11_chacha20poly1305_unittest.cc | 13 ++++++++----- lib/softoken/pkcs11c.c | 2 +- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/gtests/pk11_gtest/pk11_chacha20poly1305_unittest.cc b/gtests/pk11_gtest/pk11_chacha20poly1305_unittest.cc index 882f1f0d2e..1bcf175c89 100644 --- a/gtests/pk11_gtest/pk11_chacha20poly1305_unittest.cc +++ b/gtests/pk11_gtest/pk11_chacha20poly1305_unittest.cc @@ -261,13 +261,16 @@ TEST_F(Pkcs11ChaCha20Poly1305Test, GenerateXor) { ScopedPK11SymKey key(PK11_KeyGen(slot.get(), kMech, nullptr, 32, nullptr)); EXPECT_TRUE(!!key); - SECItem ctrNonceItem = {siBuffer, toUcharPtr(kCtrNonce), - static_cast(sizeof(kCtrNonce))}; + std::vector iv(16); + SECStatus rv = PK11_GenerateRandomOnSlot(slot.get(), iv.data(), iv.size()); + EXPECT_EQ(SECSuccess, rv); + + SECItem ctrNonceItem = {siBuffer, toUcharPtr(iv.data()), + static_cast(iv.size())}; uint8_t encrypted[sizeof(kData)]; unsigned int encrypted_len = 88; // This should be overwritten. - SECStatus rv = - PK11_Encrypt(key.get(), kMechXor, &ctrNonceItem, encrypted, - &encrypted_len, sizeof(encrypted), kData, sizeof(kData)); + rv = PK11_Encrypt(key.get(), kMechXor, &ctrNonceItem, encrypted, + &encrypted_len, sizeof(encrypted), kData, sizeof(kData)); ASSERT_EQ(SECSuccess, rv); ASSERT_EQ(sizeof(kData), static_cast(encrypted_len)); } diff --git a/lib/softoken/pkcs11c.c b/lib/softoken/pkcs11c.c index 6bc5cc9391..a9cf0cd9cf 100644 --- a/lib/softoken/pkcs11c.c +++ b/lib/softoken/pkcs11c.c @@ -1238,7 +1238,7 @@ sftk_CryptInit(CK_SESSION_HANDLE hSession, CK_MECHANISM_PTR pMechanism, PRUint8 *param = pMechanism->pParameter; int i = 0; for (; i < 4; ++i) { - ctx->counter |= param[i] << (i * 8); + ctx->counter |= (PRUint32)param[i] << (i * 8); } memcpy(ctx->nonce, param + 4, 12); context->cipherInfo = ctx;