Skip to content

Commit

Permalink
Bug 1603257 - Fix UBSAN issue in softoken CKM_NSS_CHACHA20_CTR initia…
Browse files Browse the repository at this point in the history
…lization 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
  • Loading branch information
Kevin Jacobs committed Dec 12, 2019
1 parent e0a8d0e commit f699f5c
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 6 deletions.
13 changes: 8 additions & 5 deletions gtests/pk11_gtest/pk11_chacha20poly1305_unittest.cc
Expand Up @@ -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<unsigned int>(sizeof(kCtrNonce))};
std::vector<uint8_t> iv(16);
SECStatus rv = PK11_GenerateRandomOnSlot(slot.get(), iv.data(), iv.size());
EXPECT_EQ(SECSuccess, rv);

SECItem ctrNonceItem = {siBuffer, toUcharPtr(iv.data()),
static_cast<unsigned int>(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<size_t>(encrypted_len));
}
Expand Down
2 changes: 1 addition & 1 deletion lib/softoken/pkcs11c.c
Expand Up @@ -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;
Expand Down

0 comments on commit f699f5c

Please sign in to comment.