From a5ed762d82bc7cf0b44026d073d60cef3219ed2c Mon Sep 17 00:00:00 2001 From: James M Snell Date: Sun, 19 Jan 2025 08:55:54 -0800 Subject: [PATCH] deps: fixup some minor coverity warnings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes: https://github.com/nodejs/node/issues/56611 PR-URL: https://github.com/nodejs/node/pull/56612 Reviewed-By: Michaël Zasso Reviewed-By: Benjamin Gruenbaum Reviewed-By: Yagiz Nizipli Reviewed-By: Michael Dawson Reviewed-By: Ulises Gascón --- deps/ncrypto/ncrypto.cc | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/deps/ncrypto/ncrypto.cc b/deps/ncrypto/ncrypto.cc index fa0cf58062d897..ce2e7b384eb198 100644 --- a/deps/ncrypto/ncrypto.cc +++ b/deps/ncrypto/ncrypto.cc @@ -1346,8 +1346,11 @@ DHPointer DHPointer::New(BignumPointer&& p, BignumPointer&& g) { if (DH_set0_pqg(dh.get(), p.get(), nullptr, g.get()) != 1) return {}; // If the call above is successful, the DH object takes ownership of the - // BIGNUMs, so we must release them here. + // BIGNUMs, so we must release them here. Unfortunately coverity does not + // know that so we need to tell it not to complain. + // coverity[resource_leak] p.release(); + // coverity[resource_leak] g.release(); return dh; @@ -1430,7 +1433,10 @@ DataPointer DHPointer::generateKeys() const { size_t DHPointer::size() const { if (!dh_) return 0; - return DH_size(dh_.get()); + int ret = DH_size(dh_.get()); + // DH_size can return a -1 on error but we just want to return a 0 + // in that case so we don't wrap around when returning the size_t. + return ret >= 0 ? static_cast(ret) : 0; } DataPointer DHPointer::computeSecret(const BignumPointer& peer) const { @@ -1459,6 +1465,10 @@ DataPointer DHPointer::computeSecret(const BignumPointer& peer) const { bool DHPointer::setPublicKey(BignumPointer&& key) { if (!dh_) return false; if (DH_set0_key(dh_.get(), key.get(), nullptr) == 1) { + // If DH_set0_key returns successfully, then dh_ takes ownership of the + // BIGNUM, so we must release it here. Unfortunately coverity does not + // know that so we need to tell it not to complain. + // coverity[resource_leak] key.release(); return true; } @@ -1468,6 +1478,10 @@ bool DHPointer::setPublicKey(BignumPointer&& key) { bool DHPointer::setPrivateKey(BignumPointer&& key) { if (!dh_) return false; if (DH_set0_key(dh_.get(), nullptr, key.get()) == 1) { + // If DH_set0_key returns successfully, then dh_ takes ownership of the + // BIGNUM, so we must release it here. Unfortunately coverity does not + // know that so we need to tell it not to complain. + // coverity[resource_leak] key.release(); return true; }