From 49c1657c15ca0a4309e2c8036eabc94fdbfc04ac Mon Sep 17 00:00:00 2001 From: Nickolay Olshevsky Date: Wed, 23 Oct 2024 13:15:03 +0300 Subject: [PATCH 1/4] Fix oss-fuzz issue 71368: invalid enum value. --- src/lib/crypto/sphincsplus.cpp | 2 +- src/lib/crypto/sphincsplus.h | 2 +- src/librepgp/stream-sig.cpp | 10 +++++++--- .../invalid-enum-value-4717481657171968 | Bin 0 -> 46 bytes src/tests/fuzz_verify_detached.cpp | 3 +++ 5 files changed, 12 insertions(+), 5 deletions(-) create mode 100644 src/tests/data/test_fuzz_verify_detached/invalid-enum-value-4717481657171968 diff --git a/src/lib/crypto/sphincsplus.cpp b/src/lib/crypto/sphincsplus.cpp index 5e664b42ac..c89ce70787 100644 --- a/src/lib/crypto/sphincsplus.cpp +++ b/src/lib/crypto/sphincsplus.cpp @@ -324,7 +324,7 @@ sphincsplus_signature_size(sphincsplus_parameter_t param) return 49856; default: RNP_LOG("invalid SLH-DSA parameter identifier"); - throw rnp::rnp_exception(RNP_ERROR_BAD_PARAMETERS); + return 0; } } diff --git a/src/lib/crypto/sphincsplus.h b/src/lib/crypto/sphincsplus.h index 17d61c8e22..95300f09a4 100644 --- a/src/lib/crypto/sphincsplus.h +++ b/src/lib/crypto/sphincsplus.h @@ -39,7 +39,7 @@ struct pgp_sphincsplus_key_t; struct pgp_sphincsplus_signature_t; typedef enum { sphincsplus_sha256, sphinscplus_shake256 } sphincsplus_hash_func_t; -typedef enum { +typedef enum : uint8_t { sphincsplus_simple_128s = 1, sphincsplus_simple_128f = 2, sphincsplus_simple_192s = 3, diff --git a/src/librepgp/stream-sig.cpp b/src/librepgp/stream-sig.cpp index b91cd67f8c..977aef1a9b 100644 --- a/src/librepgp/stream-sig.cpp +++ b/src/librepgp/stream-sig.cpp @@ -1102,10 +1102,14 @@ pgp_signature_t::parse_material(pgp_signature_material_t &material) const RNP_LOG("failed to parse SLH-DSA signature data"); return false; } + auto sig_size = sphincsplus_signature_size((sphincsplus_parameter_t) param); + if (!sig_size) { + RNP_LOG("invalid SLH-DSA param value"); + return false; + } material.sphincsplus.param = (sphincsplus_parameter_t) param; - material.sphincsplus.sig.resize( - sphincsplus_signature_size(material.sphincsplus.param)); - if (!pkt.get(material.sphincsplus.sig.data(), material.sphincsplus.sig.size())) { + material.sphincsplus.sig.resize(sig_size); + if (!pkt.get(material.sphincsplus.sig.data(), sig_size)) { RNP_LOG("failed to parse SLH-DSA signature data"); return false; } diff --git a/src/tests/data/test_fuzz_verify_detached/invalid-enum-value-4717481657171968 b/src/tests/data/test_fuzz_verify_detached/invalid-enum-value-4717481657171968 new file mode 100644 index 0000000000000000000000000000000000000000..5e33981ab4d24437ecb43d4063aace9d009d9587 GIT binary patch literal 46 hcmeCsVNu9cU|?X7VPaQc)D%-xK!E@M|06hH767%22uT0{ literal 0 HcmV?d00001 diff --git a/src/tests/fuzz_verify_detached.cpp b/src/tests/fuzz_verify_detached.cpp index 37bb97603d..b7d4e4e657 100644 --- a/src/tests/fuzz_verify_detached.cpp +++ b/src/tests/fuzz_verify_detached.cpp @@ -44,4 +44,7 @@ TEST_F(rnp_tests, test_fuzz_verify_detached) data = file_to_vec(DATA_PATH "outofmemory-dea88a4aa4ab5fec1291446db702ee893d5559cf"); assert_int_equal(verify_detached_LLVMFuzzerTestOneInput(data.data(), data.size()), 0); + + data = file_to_vec(DATA_PATH "invalid-enum-value-4717481657171968"); + assert_int_equal(verify_detached_LLVMFuzzerTestOneInput(data.data(), data.size()), 0); } From 9085c736815fb8cc25cb44ce1e4475b7a441dd42 Mon Sep 17 00:00:00 2001 From: Nickolay Olshevsky Date: Wed, 23 Oct 2024 13:26:32 +0300 Subject: [PATCH 2/4] Fix oss-fuzz issue 71399: invalid param value --- src/lib/crypto/sphincsplus.cpp | 2 +- src/lib/key_material.cpp | 7 ++++++- .../data/test_fuzz_dump/abrt-5093675862917120 | Bin 0 -> 155 bytes src/tests/fuzz_dump.cpp | 3 +++ 4 files changed, 10 insertions(+), 2 deletions(-) create mode 100644 src/tests/data/test_fuzz_dump/abrt-5093675862917120 diff --git a/src/lib/crypto/sphincsplus.cpp b/src/lib/crypto/sphincsplus.cpp index c89ce70787..7b2ef80af6 100644 --- a/src/lib/crypto/sphincsplus.cpp +++ b/src/lib/crypto/sphincsplus.cpp @@ -302,7 +302,7 @@ sphincsplus_pubkey_size(sphincsplus_parameter_t param) return 64; default: RNP_LOG("invalid SLH-DSA parameter identifier"); - throw rnp::rnp_exception(RNP_ERROR_BAD_PARAMETERS); + return 0; } } diff --git a/src/lib/key_material.cpp b/src/lib/key_material.cpp index 9babe1275e..e989e8eb36 100644 --- a/src/lib/key_material.cpp +++ b/src/lib/key_material.cpp @@ -1823,7 +1823,12 @@ SlhdsaKeyMaterial::parse(pgp_packet_body_t &pkt) noexcept return false; } sphincsplus_parameter_t param = (sphincsplus_parameter_t) bt; - std::vector buf(sphincsplus_pubkey_size(param)); + auto size = sphincsplus_pubkey_size(param); + if (!size) { + RNP_LOG("invalid SLH-DSA param"); + return false; + } + std::vector buf(size); if (!pkt.get(buf.data(), buf.size())) { RNP_LOG("failed to parse SLH-DSA public key data"); return false; diff --git a/src/tests/data/test_fuzz_dump/abrt-5093675862917120 b/src/tests/data/test_fuzz_dump/abrt-5093675862917120 new file mode 100644 index 0000000000000000000000000000000000000000..fd29cb1f4346c31b70926d784d831d93f36882dc GIT binary patch literal 155 zcmX>e$|}JikeHianpeWZomx~p;p&5Z|AF8bOMc|P$P=F!8Q2*Z7?>IUF#qSbW?*n) zRAm3Rj^Y2$sf^Z#8Qf3Jzq>tv1*m|5AxITSJcfY7j~_cSH$b^CPH{nM@(vy@e0Kal Ov5Ar4;X{yd3=9A|mO{(` literal 0 HcmV?d00001 diff --git a/src/tests/fuzz_dump.cpp b/src/tests/fuzz_dump.cpp index f24921682a..a193c00d0a 100644 --- a/src/tests/fuzz_dump.cpp +++ b/src/tests/fuzz_dump.cpp @@ -58,4 +58,7 @@ TEST_F(rnp_tests, test_fuzz_dump) data = file_to_vec(DATA_PATH "outofmemory-6111789935624192"); assert_int_equal(dump_LLVMFuzzerTestOneInput(data.data(), data.size()), 0); + + data = file_to_vec(DATA_PATH "abrt-5093675862917120"); + assert_int_equal(dump_LLVMFuzzerTestOneInput(data.data(), data.size()), 0); } From 7db1f238d1833902d9ec79f06d734ed6c5a250ed Mon Sep 17 00:00:00 2001 From: Nickolay Olshevsky Date: Wed, 23 Oct 2024 13:33:34 +0300 Subject: [PATCH 3/4] Update fuzzers code to avoid coverity USE_AFTER_FREE false-positives. --- src/fuzzing/dump.c | 7 ++++--- src/fuzzing/keyimport.c | 39 ++++++++++++++++----------------------- 2 files changed, 20 insertions(+), 26 deletions(-) diff --git a/src/fuzzing/dump.c b/src/fuzzing/dump.c index 026bfc2503..356a239f1e 100644 --- a/src/fuzzing/dump.c +++ b/src/fuzzing/dump.c @@ -44,11 +44,12 @@ LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) rnp_output_destroy(output); rnp_input_destroy(input); - (void) rnp_input_from_memory(&input, data, size, false); + rnp_input_t input2 = NULL; + (void) rnp_input_from_memory(&input2, data, size, false); char *json = NULL; - (void) rnp_dump_packets_to_json(input, RNP_DUMP_RAW, &json); + (void) rnp_dump_packets_to_json(input2, RNP_DUMP_RAW, &json); rnp_buffer_destroy(json); - rnp_input_destroy(input); + rnp_input_destroy(input2); return 0; } diff --git a/src/fuzzing/keyimport.c b/src/fuzzing/keyimport.c index 16e1272f8c..139c62841f 100644 --- a/src/fuzzing/keyimport.c +++ b/src/fuzzing/keyimport.c @@ -39,58 +39,51 @@ LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) rnp_input_t input = NULL; rnp_result_t ret = 0; rnp_ffi_t ffi = NULL; + uint32_t flags = RNP_LOAD_SAVE_PUBLIC_KEYS | RNP_LOAD_SAVE_SECRET_KEYS; /* try non-permissive import */ ret = rnp_input_from_memory(&input, data, size, false); ret = rnp_ffi_create(&ffi, "GPG", "GPG"); char *results = NULL; - ret = rnp_import_keys( - ffi, input, RNP_LOAD_SAVE_PUBLIC_KEYS | RNP_LOAD_SAVE_SECRET_KEYS, &results); + ret = rnp_import_keys(ffi, input, flags, &results); rnp_buffer_destroy(results); rnp_input_destroy(input); rnp_ffi_destroy(ffi); /* try permissive import */ - ret = rnp_input_from_memory(&input, data, size, false); + rnp_input_t input2 = NULL; + ret = rnp_input_from_memory(&input2, data, size, false); ret = rnp_ffi_create(&ffi, "GPG", "GPG"); results = NULL; - ret = rnp_import_keys(ffi, - input, - RNP_LOAD_SAVE_PUBLIC_KEYS | RNP_LOAD_SAVE_SECRET_KEYS | - RNP_LOAD_SAVE_PERMISSIVE, - &results); + ret = rnp_import_keys(ffi, input2, flags | RNP_LOAD_SAVE_PERMISSIVE, &results); rnp_buffer_destroy(results); - rnp_input_destroy(input); + rnp_input_destroy(input2); rnp_ffi_destroy(ffi); /* try non-permissive iterative import */ - ret = rnp_input_from_memory(&input, data, size, false); + rnp_input_t input3 = NULL; + ret = rnp_input_from_memory(&input3, data, size, false); ret = rnp_ffi_create(&ffi, "GPG", "GPG"); + flags |= RNP_LOAD_SAVE_SINGLE; do { results = NULL; - ret = rnp_import_keys(ffi, - input, - RNP_LOAD_SAVE_PUBLIC_KEYS | RNP_LOAD_SAVE_SECRET_KEYS | - RNP_LOAD_SAVE_SINGLE, - &results); + ret = rnp_import_keys(ffi, input3, flags, &results); rnp_buffer_destroy(results); } while (!ret); - rnp_input_destroy(input); + rnp_input_destroy(input3); rnp_ffi_destroy(ffi); /* try permissive iterative import */ - ret = rnp_input_from_memory(&input, data, size, false); + rnp_input_t input4 = NULL; + ret = rnp_input_from_memory(&input4, data, size, false); ret = rnp_ffi_create(&ffi, "GPG", "GPG"); + flags |= RNP_LOAD_SAVE_PERMISSIVE; do { results = NULL; - ret = rnp_import_keys(ffi, - input, - RNP_LOAD_SAVE_PUBLIC_KEYS | RNP_LOAD_SAVE_SECRET_KEYS | - RNP_LOAD_SAVE_PERMISSIVE | RNP_LOAD_SAVE_SINGLE, - &results); + ret = rnp_import_keys(ffi, input4, flags, &results); rnp_buffer_destroy(results); } while (!ret); - rnp_input_destroy(input); + rnp_input_destroy(input4); rnp_ffi_destroy(ffi); return 0; From 064c77a13550f1d669f3913f51b1ddb49b5dca77 Mon Sep 17 00:00:00 2001 From: Nickolay Olshevsky Date: Wed, 23 Oct 2024 13:49:30 +0300 Subject: [PATCH 4/4] Fix bunch of minor coverity issues. --- src/lib/pgp-key.cpp | 2 +- src/lib/sig_subpacket.cpp | 4 ++-- src/lib/sig_subpacket.hpp | 5 +++-- src/librepgp/stream-sig.cpp | 3 +++ src/librepgp/stream-sig.h | 4 ++-- 5 files changed, 11 insertions(+), 7 deletions(-) diff --git a/src/lib/pgp-key.cpp b/src/lib/pgp-key.cpp index 3897f26931..b4c9f0551b 100644 --- a/src/lib/pgp-key.cpp +++ b/src/lib/pgp-key.cpp @@ -2097,7 +2097,7 @@ pgp_key_t::validate_sig(pgp_signature_info_t & sinfo, if (!subpkt->critical() || (subpkt->type() != pgp::pkt::sigsub::Type::NotationData)) { continue; } - auto notation = dynamic_cast(*subpkt); + auto ¬ation = dynamic_cast(*subpkt); RNP_LOG("unknown critical notation: %s", notation.name().c_str()); sinfo.valid = false; } diff --git a/src/lib/sig_subpacket.cpp b/src/lib/sig_subpacket.cpp index ebbffcaa3a..b13ab6b75a 100644 --- a/src/lib/sig_subpacket.cpp +++ b/src/lib/sig_subpacket.cpp @@ -672,11 +672,11 @@ bool EmbeddedSignature::parse_data(const uint8_t *data, size_t size) { pgp_packet_body_t pkt(data, size); - pgp_signature_t sig{}; + pgp_signature_t sig; if (sig.parse(pkt)) { return false; } - signature_ = std::unique_ptr(new pgp_signature_t(sig)); + signature_ = std::unique_ptr(new pgp_signature_t(std::move(sig))); return true; } diff --git a/src/lib/sig_subpacket.hpp b/src/lib/sig_subpacket.hpp index 7a8aa4133b..82732545af 100644 --- a/src/lib/sig_subpacket.hpp +++ b/src/lib/sig_subpacket.hpp @@ -471,7 +471,8 @@ class RevocationKey : public Raw { public: RevocationKey(bool hashed = true, bool critical = false) - : Raw(Type::RevocationKey, hashed, critical){}; + : Raw(Type::RevocationKey, hashed, critical), rev_class_(0), + alg_(PGP_PKA_NOTHING), fp_{} {}; uint8_t rev_class() const noexcept @@ -866,7 +867,7 @@ class IssuerFingerprint : public Raw { public: IssuerFingerprint(bool hashed = true, bool critical = false) - : Raw(Type::IssuerFingerprint, hashed, critical), version_(0) + : Raw(Type::IssuerFingerprint, hashed, critical), version_(0), fp_{} { } diff --git a/src/librepgp/stream-sig.cpp b/src/librepgp/stream-sig.cpp index 977aef1a9b..9a1a5422be 100644 --- a/src/librepgp/stream-sig.cpp +++ b/src/librepgp/stream-sig.cpp @@ -432,6 +432,9 @@ pgp_signature_t::set_preferred(const std::vector &data, sigsub::Type ty auto sub = sigsub::Raw::create(type); auto pref = dynamic_cast(sub.get()); + if (!pref) { + return; + } pref->set_algs(data); add_subpkt(std::move(sub)); } diff --git a/src/librepgp/stream-sig.h b/src/librepgp/stream-sig.h index 8ac5a9dd9d..398f9daf60 100644 --- a/src/librepgp/stream-sig.h +++ b/src/librepgp/stream-sig.h @@ -53,13 +53,13 @@ typedef struct pgp_signature_t { /* common v3 and v4 fields */ pgp_pubkey_alg_t palg; pgp_hash_alg_t halg; - std::array lbits; + std::array lbits{}; std::vector hashed_data; std::vector material_buf; /* raw signature material */ /* v3 - only fields */ uint32_t creation_time; - pgp_key_id_t signer; + pgp_key_id_t signer{}; /* common v4, v5 and v6 fields */ pgp::pkt::sigsub::List subpkts;