From 7175dcb1078abbdaa130a8c5f5fd2b93fa7b3086 Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Wed, 24 Jun 2020 23:26:58 +0000 Subject: [PATCH] replace most boost serialization with existing monero serialization This reduces the attack surface for data that can come from malicious sources (exported output and key images, multisig transactions...) since the monero serialization is already exposed to the outside, and the boost lib we were using had a few known crashers. For interoperability, a new load-deprecated-formats wallet setting is added (off by default). This allows loading boost format data if there is no alternative. It will likely go at some point, along with the ability to load those. Notably, the peer lists file still uses the boost serialization code, as the data it stores is define in epee, while the new serialization code is in monero, and migrating it was fairly hairy. Since this file is local and not obtained from anyone else, the marginal risk is minimal, but it could be migrated later if needed. Some tests and tools also do, this will stay as is for now. --- .../blockchain_blackball.cpp | 2 - src/cryptonote_basic/cryptonote_basic.h | 2 +- .../cryptonote_basic_impl.cpp | 1 - .../cryptonote_boost_serialization.h | 1 - src/p2p/p2p_protocol_defs.h | 22 ++ src/ringct/rctTypes.h | 29 ++- src/rpc/rpc_payment.cpp | 29 ++- src/rpc/rpc_payment.h | 57 ++++- src/serialization/container.h | 6 +- src/serialization/containers.h | 128 ++++++++++ src/serialization/deque.h | 64 ----- src/serialization/list.h | 2 +- src/serialization/serialization.h | 22 ++ src/serialization/set.h | 58 ----- src/serialization/unordered_set.h | 58 ----- src/serialization/vector.h | 65 ----- src/simplewallet/simplewallet.cpp | 24 ++ src/simplewallet/simplewallet.h | 1 + src/wallet/message_store.cpp | 83 ++++-- src/wallet/message_store.h | 77 +++++- src/wallet/wallet2.cpp | 241 ++++++++++++++---- src/wallet/wallet2.h | 203 ++++++++++++--- src/wallet/wallet_rpc_server.cpp | 28 +- src/wallet/wallet_rpc_server_commands_defs.h | 2 +- tests/core_tests/chaingen.h | 2 - tests/data/fuzz/cold-outputs/OUTPUTS1 | Bin 7 -> 0 bytes tests/data/fuzz/cold-outputs/OUTPUTS2 | Bin 581 -> 0 bytes tests/data/fuzz/cold-outputs/out-all-6 | Bin 0 -> 2607 bytes tests/data/fuzz/cold-outputs/out-none-6 | Bin 0 -> 3 bytes tests/data/fuzz/cold-transaction/CTX1 | Bin 1073 -> 4192 bytes tests/fuzz/cold-outputs.cpp | 8 +- tests/fuzz/cold-transaction.cpp | 8 +- tests/unit_tests/serialization.cpp | 2 +- tests/unit_tests/varint.cpp | 2 +- 34 files changed, 836 insertions(+), 391 deletions(-) create mode 100644 src/serialization/containers.h delete mode 100644 src/serialization/deque.h delete mode 100644 src/serialization/set.h delete mode 100644 src/serialization/unordered_set.h delete mode 100644 src/serialization/vector.h delete mode 100644 tests/data/fuzz/cold-outputs/OUTPUTS1 delete mode 100644 tests/data/fuzz/cold-outputs/OUTPUTS2 create mode 100644 tests/data/fuzz/cold-outputs/out-all-6 create mode 100644 tests/data/fuzz/cold-outputs/out-none-6 diff --git a/src/blockchain_utilities/blockchain_blackball.cpp b/src/blockchain_utilities/blockchain_blackball.cpp index a847f6a9d..a8197483f 100644 --- a/src/blockchain_utilities/blockchain_blackball.cpp +++ b/src/blockchain_utilities/blockchain_blackball.cpp @@ -28,8 +28,6 @@ #include #include -#include -#include #include "common/unordered_containers_boost_serialization.h" #include "common/command_line.h" #include "common/varint.h" diff --git a/src/cryptonote_basic/cryptonote_basic.h b/src/cryptonote_basic/cryptonote_basic.h index bc6a378f2..c70ae1df1 100644 --- a/src/cryptonote_basic/cryptonote_basic.h +++ b/src/cryptonote_basic/cryptonote_basic.h @@ -37,7 +37,7 @@ #include #include #include "serialization/variant.h" -#include "serialization/vector.h" +#include "serialization/containers.h" #include "serialization/binary_archive.h" #include "serialization/json_archive.h" #include "serialization/debug_archive.h" diff --git a/src/cryptonote_basic/cryptonote_basic_impl.cpp b/src/cryptonote_basic/cryptonote_basic_impl.cpp index 541393fa9..2600854a9 100644 --- a/src/cryptonote_basic/cryptonote_basic_impl.cpp +++ b/src/cryptonote_basic/cryptonote_basic_impl.cpp @@ -34,7 +34,6 @@ using namespace epee; #include "cryptonote_basic_impl.h" #include "string_tools.h" #include "serialization/binary_utils.h" -#include "serialization/container.h" #include "cryptonote_format_utils.h" #include "cryptonote_config.h" #include "misc_language.h" diff --git a/src/cryptonote_basic/cryptonote_boost_serialization.h b/src/cryptonote_basic/cryptonote_boost_serialization.h index 6f89cc7ae..b3d39a616 100644 --- a/src/cryptonote_basic/cryptonote_boost_serialization.h +++ b/src/cryptonote_basic/cryptonote_boost_serialization.h @@ -36,7 +36,6 @@ #include #include #include -#include #include #include #include "cryptonote_basic.h" diff --git a/src/p2p/p2p_protocol_defs.h b/src/p2p/p2p_protocol_defs.h index bbc165cfa..b439dc47e 100644 --- a/src/p2p/p2p_protocol_defs.h +++ b/src/p2p/p2p_protocol_defs.h @@ -39,6 +39,7 @@ #include "misc_language.h" #include "string_tools.h" #include "time_helper.h" +#include "serialization/serialization.h" #include "cryptonote_config.h" namespace nodetool @@ -84,6 +85,15 @@ namespace nodetool KV_SERIALIZE_OPT(rpc_port, (uint16_t)0) KV_SERIALIZE_OPT(rpc_credits_per_hash, (uint32_t)0) END_KV_SERIALIZE_MAP() + + BEGIN_SERIALIZE() + FIELD(adr) + FIELD(id) + VARINT_FIELD(last_seen) + VARINT_FIELD(pruning_seed) + VARINT_FIELD(rpc_port) + VARINT_FIELD(rpc_credits_per_hash) + END_SERIALIZE() }; typedef peerlist_entry_base peerlist_entry; @@ -99,6 +109,12 @@ namespace nodetool KV_SERIALIZE(id) KV_SERIALIZE(first_seen) END_KV_SERIALIZE_MAP() + + BEGIN_SERIALIZE() + FIELD(adr) + FIELD(id) + VARINT_FIELD(first_seen) + END_SERIALIZE() }; typedef anchor_peerlist_entry_base anchor_peerlist_entry; @@ -114,6 +130,12 @@ namespace nodetool KV_SERIALIZE(id) KV_SERIALIZE(is_income) END_KV_SERIALIZE_MAP() + + BEGIN_SERIALIZE() + FIELD(adr) + FIELD(id) + FIELD(is_income) + END_SERIALIZE() }; typedef connection_entry_base connection_entry; diff --git a/src/ringct/rctTypes.h b/src/ringct/rctTypes.h index 9b7f26a02..ce11981ad 100644 --- a/src/ringct/rctTypes.h +++ b/src/ringct/rctTypes.h @@ -49,7 +49,7 @@ extern "C" { #include "hex.h" #include "span.h" #include "memwipe.h" -#include "serialization/vector.h" +#include "serialization/containers.h" #include "serialization/debug_archive.h" #include "serialization/binary_archive.h" #include "serialization/json_archive.h" @@ -239,6 +239,12 @@ namespace rct { struct RCTConfig { RangeProofType range_proof_type; int bp_version; + + BEGIN_SERIALIZE_OBJECT() + VERSION_FIELD(0) + VARINT_FIELD(range_proof_type) + VARINT_FIELD(bp_version) + END_SERIALIZE() }; struct rctSigBase { uint8_t type; @@ -317,6 +323,16 @@ namespace rct { ar.end_array(); return ar.stream().good(); } + + BEGIN_SERIALIZE_OBJECT() + FIELD(type) + FIELD(message) + FIELD(mixRing) + FIELD(pseudoOuts) + FIELD(ecdhInfo) + FIELD(outPk) + VARINT_FIELD(txnFee) + END_SERIALIZE() }; struct rctSigPrunable { std::vector rangeSigs; @@ -436,6 +452,12 @@ namespace rct { return ar.stream().good(); } + BEGIN_SERIALIZE_OBJECT() + FIELD(rangeSigs) + FIELD(bulletproofs) + FIELD(MGs) + FIELD(pseudoOuts) + END_SERIALIZE() }; struct rctSig: public rctSigBase { rctSigPrunable p; @@ -449,6 +471,11 @@ namespace rct { { return type == RCTTypeBulletproof || type == RCTTypeBulletproof2 ? p.pseudoOuts : pseudoOuts; } + + BEGIN_SERIALIZE_OBJECT() + FIELDS((rctSigBase&)*this) + FIELD(p) + END_SERIALIZE() }; //other basepoint H = toPoint(cn_fast_hash(G)), G the basepoint diff --git a/src/rpc/rpc_payment.cpp b/src/rpc/rpc_payment.cpp index edc8f0dda..176f11fa3 100644 --- a/src/rpc/rpc_payment.cpp +++ b/src/rpc/rpc_payment.cpp @@ -27,14 +27,12 @@ // THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #include -#include #include "cryptonote_config.h" #include "include_base_utils.h" #include "string_tools.h" #include "file_io_utils.h" #include "int-util.h" #include "common/util.h" -#include "serialization/crypto.h" #include "common/unordered_containers_boost_serialization.h" #include "cryptonote_basic/cryptonote_boost_serialization.h" #include "cryptonote_basic/cryptonote_format_utils.h" @@ -296,14 +294,28 @@ namespace cryptonote data.open(state_file_path, std::ios_base::binary | std::ios_base::in); if (!data.fail()) { + bool loaded = false; try { - boost::archive::portable_binary_iarchive a(data); - a >> *this; + binary_archive ar(data); + if (::serialization::serialize(ar, *this)) + if (::serialization::check_stream_state(ar)) + loaded = true; } - catch (const std::exception &e) + catch (...) {} + if (!loaded) { - MERROR("Failed to load RPC payments file: " << e.what()); + try + { + boost::archive::portable_binary_iarchive a(data); + a >> *this; + loaded = true; + } + catch (...) {} + } + if (!loaded) + { + MERROR("Failed to load RPC payments file"); m_client_info.clear(); } } @@ -344,8 +356,9 @@ namespace cryptonote MWARNING("Failed to save RPC payments to file " << state_file_path); return false; }; - boost::archive::portable_binary_oarchive a(data); - a << *this; + binary_archive ar(data); + if (!::serialization::serialize(ar, *const_cast(this))) + return false; return true; CATCH_ENTRY_L0("rpc_payment::store", false); } diff --git a/src/rpc/rpc_payment.h b/src/rpc/rpc_payment.h index dcd43f8d5..fdf1f953f 100644 --- a/src/rpc/rpc_payment.h +++ b/src/rpc/rpc_payment.h @@ -31,10 +31,17 @@ #include #include #include +#include #include #include #include "cryptonote_basic/blobdatatype.h" #include "cryptonote_basic/cryptonote_basic.h" +#include +#include +#include "serialization/crypto.h" +#include "serialization/string.h" +#include "serialization/pair.h" +#include "serialization/containers.h" namespace cryptonote { @@ -96,6 +103,33 @@ namespace cryptonote a & nonces_bad; a & nonces_dupe; } + + BEGIN_SERIALIZE_OBJECT() + VERSION_FIELD(0) + FIELD(block) + FIELD(previous_block) + FIELD(hashing_blob) + FIELD(previous_hashing_blob) + VARINT_FIELD(seed_height) + VARINT_FIELD(previous_seed_height) + FIELD(seed_hash) + FIELD(previous_seed_hash) + VARINT_FIELD(cookie) + FIELD(top) + FIELD(previous_top) + VARINT_FIELD(credits) + FIELD(payments) + FIELD(previous_payments) + FIELD(update_time) + FIELD(last_request_timestamp) + FIELD(block_template_update_time) + VARINT_FIELD(credits_total) + VARINT_FIELD(credits_used) + VARINT_FIELD(nonces_good) + VARINT_FIELD(nonces_stale) + VARINT_FIELD(nonces_bad) + VARINT_FIELD(nonces_dupe) + END_SERIALIZE() }; public: @@ -114,8 +148,8 @@ namespace cryptonote template inline void serialize(t_archive &a, const unsigned int ver) { - a & m_client_info; - a & m_hashrate; + a & m_client_info.parent(); + a & m_hashrate.parent(); a & m_credits_total; a & m_credits_used; a & m_nonces_good; @@ -124,6 +158,18 @@ namespace cryptonote a & m_nonces_dupe; } + BEGIN_SERIALIZE_OBJECT() + VERSION_FIELD(0) + FIELD(m_client_info) + FIELD(m_hashrate) + VARINT_FIELD(m_credits_total) + VARINT_FIELD(m_credits_used) + VARINT_FIELD(m_nonces_good) + VARINT_FIELD(m_nonces_stale) + VARINT_FIELD(m_nonces_bad) + VARINT_FIELD(m_nonces_dupe) + END_SERIALIZE() + bool load(std::string directory); bool store(const std::string &directory = std::string()) const; @@ -131,9 +177,9 @@ namespace cryptonote cryptonote::account_public_address m_address; uint64_t m_diff; uint64_t m_credits_per_hash_found; - std::unordered_map m_client_info; + serializable_unordered_map m_client_info; std::string m_directory; - std::map m_hashrate; + serializable_map m_hashrate; uint64_t m_credits_total; uint64_t m_credits_used; uint64_t m_nonces_good; @@ -143,6 +189,3 @@ namespace cryptonote mutable boost::mutex mutex; }; } - -BOOST_CLASS_VERSION(cryptonote::rpc_payment, 0); -BOOST_CLASS_VERSION(cryptonote::rpc_payment::client_info, 0); diff --git a/src/serialization/container.h b/src/serialization/container.h index 4bf47ecfa..d5e75bb4f 100644 --- a/src/serialization/container.h +++ b/src/serialization/container.h @@ -28,10 +28,6 @@ // // Parts of this file are originally copyright (c) 2012-2013 The Cryptonote developers -#pragma once - -#include "serialization.h" - namespace serialization { namespace detail @@ -103,7 +99,7 @@ bool do_serialize_container(Archive &ar, C &v) return false; if (i != v.begin()) ar.delimit_array(); - if(!::serialization::detail::serialize_container_element(ar, const_cast(*i))) + if(!::serialization::detail::serialize_container_element(ar, (typename C::value_type&)*i)) return false; if (!ar.stream().good()) return false; diff --git a/src/serialization/containers.h b/src/serialization/containers.h new file mode 100644 index 000000000..bc4a89527 --- /dev/null +++ b/src/serialization/containers.h @@ -0,0 +1,128 @@ +// Copyright (c) 2014-2019, The Monero Project +// +// All rights reserved. +// +// Redistribution and use in source and binary forms, with or without modification, are +// permitted provided that the following conditions are met: +// +// 1. Redistributions of source code must retain the above copyright notice, this list of +// conditions and the following disclaimer. +// +// 2. Redistributions in binary form must reproduce the above copyright notice, this list +// of conditions and the following disclaimer in the documentation and/or other +// materials provided with the distribution. +// +// 3. Neither the name of the copyright holder nor the names of its contributors may be +// used to endorse or promote products derived from this software without specific +// prior written permission. +// +// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND ANY +// EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF +// MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL +// THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, +// PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS +// INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, +// STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF +// THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. +// +// Parts of this file are originally copyright (c) 2012-2013 The Cryptonote developers + +#pragma once + +#include +#include +#include +#include +#include +#include +#include "serialization.h" + +template