From f02980b67862c9c4209b4f9dd951b9b41db17914 Mon Sep 17 00:00:00 2001 From: CoraleSoft <82213665+Coralesoft@users.noreply.github.com> Date: Fri, 6 Feb 2026 12:00:45 +1300 Subject: [PATCH] Fix CVE-2024-28285 and Issue #1342 in pubkey.h CVE-2024-28285: Harden hybrid DL decryption against fault injection - Blinding verification to detect faulted key-agreement computations - Fix blinding k range Issue #1342: Fix DSA/ECDSA to handle r=0 or s=0 - Probabilistic signatures: retry with fresh random k until valid (64-attempt cap) - Deterministic signatures (RFC 6979): abort with exception (API returns single k) Fixes: https://github.com/weidai11/cryptopp/issues/1342 Fixes: https://github.com/weidai11/cryptopp/issues/1262 --- elgamal.h | 37 ++++++++++++++-- pubkey.h | 126 +++++++++++++++++++++++++++++++++++++++++++++--------- 2 files changed, 138 insertions(+), 25 deletions(-) diff --git a/elgamal.h b/elgamal.h index 7e1b78256..9a86b39c4 100644 --- a/elgamal.h +++ b/elgamal.h @@ -75,23 +75,52 @@ class CRYPTOPP_NO_VTABLE ElGamalBase : a_times_b_mod_c(Integer(key, modulusLen), Integer(block, modulusLen-1), p).Encode(cipherText, modulusLen); } + /// \brief Symmetric decryption for ElGamal DEM + /// \param key the symmetric key + /// \param cipherText the ciphertext + /// \param cipherTextLength the ciphertext length + /// \param plainText the plaintext (output) + /// \param parameters additional parameters + /// \return DecodingResult with message length on success + /// \details This function provides a no-write-on-failure guarantee as defense-in-depth. + /// The plainText buffer is only written on successful decryption. On any failure + /// the function returns DecodingResult() and the caller's buffer remains untouched. + /// \sa CVE-2024-28285 DecodingResult SymmetricDecrypt(const byte *key, const byte *cipherText, size_t cipherTextLength, byte *plainText, const NameValuePairs ¶meters) const { CRYPTOPP_UNUSED(parameters); const Integer &p = GetGroupParameters().GetModulus(); unsigned int modulusLen = p.ByteCount(); + // Validate ciphertext length if (cipherTextLength != modulusLen) return DecodingResult(); + // Compute decrypted value Integer m = a_times_b_mod_c(Integer(cipherText, modulusLen), Integer(key, modulusLen).InverseMod(p), p); - m.Encode(plainText, 1); - unsigned int plainTextLength = plainText[0]; + // CVE-2024-28285: Decode into temporary buffer to validate before writing to output + // The format is: [random padding][plaintext][length byte] + // where length byte is the last byte of the decoded message + + // Extract and validate plaintext length from the least significant byte + // Use temp buffer to avoid writing to caller's buffer on failure + SecByteBlock tempBuf(modulusLen); + m.Encode(tempBuf, modulusLen); + + // Length is in the last byte of the encoded message + unsigned int plainTextLength = tempBuf[modulusLen - 1]; if (plainTextLength > GetMaxSymmetricPlaintextLength(modulusLen)) return DecodingResult(); - m >>= 8; - m.Encode(plainText, plainTextLength); + + // Extract plaintext: it's the bytes just before the length byte + // Position: tempBuf[modulusLen - 1 - plainTextLength] to tempBuf[modulusLen - 2] + if (plainTextLength > 0) + { + size_t plainTextOffset = modulusLen - 1 - plainTextLength; + std::memcpy(plainText, tempBuf + plainTextOffset, plainTextLength); + } + return DecodingResult(plainTextLength); } diff --git a/pubkey.h b/pubkey.h index 31e00a333..10bfaf8b3 100644 --- a/pubkey.h +++ b/pubkey.h @@ -315,7 +315,7 @@ class CRYPTOPP_NO_VTABLE PK_SignatureMessageEncodingMethod virtual size_t MinRepresentativeBitLength(size_t hashIdentifierLength, size_t digestLength) const {CRYPTOPP_UNUSED(hashIdentifierLength); CRYPTOPP_UNUSED(digestLength); return 0;} virtual size_t MaxRecoverableLength(size_t representativeBitLength, size_t hashIdentifierLength, size_t digestLength) const - {CRYPTOPP_UNUSED(representativeBitLength); CRYPTOPP_UNUSED(representativeBitLength); CRYPTOPP_UNUSED(hashIdentifierLength); CRYPTOPP_UNUSED(digestLength); return 0;} + {CRYPTOPP_UNUSED(representativeBitLength); CRYPTOPP_UNUSED(hashIdentifierLength); CRYPTOPP_UNUSED(digestLength); return 0;} /// \brief Determines whether an encoding method requires a random number generator /// \return true if the encoding method requires a RandomNumberGenerator() @@ -1652,27 +1652,62 @@ class CRYPTOPP_NO_VTABLE DL_SignerBase : public DL_SignatureSchemeBase(alg); - k = det.GenerateRandom(x, q, e); + const DeterministicSignatureAlgorithm* det = + dynamic_cast(&alg); + if (!det) + throw Exception(Exception::OTHER_ERROR, + "DL_SignerBase: IsDeterministic() but algorithm is not DeterministicSignatureAlgorithm"); + k = det->GenerateRandom(x, q, e); + + // Timing side-channel mitigation for nonce length (Jancar) + // https://github.com/weidai11/cryptopp/issues/869 + ks = k + q; + if (ks.BitCount() == q.BitCount()) + ks += q; + + r = params.ConvertElementToInteger(params.ExponentiateBase(ks)); + alg.Sign(params, key.GetPrivateExponent(), k, e, r, s); + + if (r.IsZero() || s.IsZero()) + throw Exception(Exception::OTHER_ERROR, + "DL_SignerBase: deterministic signature produced r=0 or s=0"); } else { - k.Randomize(rng, 1, params.GetSubgroupOrder()-1); - } + // Probabilistic signatures: retry with fresh random k until valid. + // 64 attempts is a safety cap to avoid infinite loops under RNG failure; + // expected retries is ~1 (probability of r=0 or s=0 is ~1/q). + for (unsigned int i = 0; i < 64; ++i) + { + k.Randomize(rng, 1, q - 1); - // Due to timing attack on nonce length by Jancar - // https://github.com/weidai11/cryptopp/issues/869 - ks = k + q; - if (ks.BitCount() == q.BitCount()) { - ks += q; - } + // Timing side-channel mitigation for nonce length (Jancar) + // https://github.com/weidai11/cryptopp/issues/869 + ks = k + q; + if (ks.BitCount() == q.BitCount()) + ks += q; - Integer r, s; - r = params.ConvertElementToInteger(params.ExponentiateBase(ks)); - alg.Sign(params, key.GetPrivateExponent(), k, e, r, s); + r = params.ConvertElementToInteger(params.ExponentiateBase(ks)); + alg.Sign(params, key.GetPrivateExponent(), k, e, r, s); + + if (!r.IsZero() && !s.IsZero()) + break; + } + if (r.IsZero() || s.IsZero()) + throw Exception(Exception::OTHER_ERROR, + "DL_SignerBase: probabilistic signature failed after 64 attempts (r=0 or s=0)"); + } /* Integer r, s; @@ -1834,28 +1869,77 @@ class CRYPTOPP_NO_VTABLE DL_DecryptorBase : public DL_CryptoSystemBaseCVE-2024-28285 DecodingResult Decrypt(RandomNumberGenerator &rng, const byte *ciphertext, size_t ciphertextLength, byte *plaintext, const NameValuePairs ¶meters = g_nullNameValuePairs) const { try { - CRYPTOPP_UNUSED(rng); const DL_KeyAgreementAlgorithm &agreeAlg = this->GetKeyAgreementAlgorithm(); const DL_KeyDerivationAlgorithm &derivAlg = this->GetKeyDerivationAlgorithm(); const DL_SymmetricEncryptionAlgorithm &encAlg = this->GetSymmetricEncryptionAlgorithm(); const DL_GroupParameters ¶ms = this->GetAbstractGroupParameters(); const DL_PrivateKey &key = this->GetKeyInterface(); - Element q = params.DecodeElement(ciphertext, true); + // CVE-2024-28285: Validate ciphertext length before any processing + // Prevents out-of-bounds reads and size_t underflow size_t elementSize = params.GetEncodedElementSize(true); + if (ciphertextLength < elementSize) + return DecodingResult(); + + // Decode and validate ephemeral public key + Element ephemeralPub = params.DecodeElement(ciphertext, true); ciphertext += elementSize; ciphertextLength -= elementSize; - Element z = agreeAlg.AgreeWithStaticPrivateKey(params, q, true, key.GetPrivateExponent()); - - SecByteBlock derivedKey(encAlg.GetSymmetricKeyLength(encAlg.GetMaxSymmetricPlaintextLength(ciphertextLength))); - derivAlg.Derive(params, derivedKey, derivedKey.size(), z, q, parameters); + // Key agreement computation + const Integer &x = key.GetPrivateExponent(); + Element z = agreeAlg.AgreeWithStaticPrivateKey(params, ephemeralPub, true, x); + + // CVE-2024-28285: Blinding verification to detect faulted computations + // Compute z2 = ephemeralPub^(x + k*order) which should equal z + // For multiplicative groups: ephemeralPub^order = 1, so z2 = ephemeralPub^x * 1^k = z + // For EC groups: order*ephemeralPub = O (infinity), so z2 = x*P + k*O = z + const Integer &order = params.GetSubgroupOrder(); + Integer k(rng, Integer::One(), order - 1); + Integer blindExp = x + k * order; + Element z2 = params.ExponentiateElement(ephemeralPub, blindExp); + if (!(z == z2)) + return DecodingResult(); // Fault detected, reject decryption + + // Derive symmetric key + size_t derivedKeyLen = encAlg.GetSymmetricKeyLength(encAlg.GetMaxSymmetricPlaintextLength(ciphertextLength)); + SecByteBlock derivedKey(derivedKeyLen); + derivAlg.Derive(params, derivedKey, derivedKey.size(), z, ephemeralPub, parameters); + + // CVE-2024-28285: Decrypt into temporary buffer first + // Only copy to caller's buffer on success (no-write-on-failure guarantee) + // Use ciphertextLength as buffer size - it's always >= plaintext length + size_t tempBufferLen = ciphertextLength > 0 ? ciphertextLength : 1; + SecByteBlock tempPlaintext(tempBufferLen); + + DecodingResult result = encAlg.SymmetricDecrypt(derivedKey, ciphertext, ciphertextLength, tempPlaintext, parameters); + + if (result.isValidCoding && result.messageLength <= tempBufferLen) + { + // Success: copy plaintext to caller's buffer + std::memcpy(plaintext, tempPlaintext, result.messageLength); + } - return encAlg.SymmetricDecrypt(derivedKey, ciphertext, ciphertextLength, plaintext, parameters); + return result; } catch (DL_BadElement &) {