From 0c36b9f93128cd814fabac34a797d3fbd02d97b6 Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Wed, 13 Sep 2017 09:22:16 +0100 Subject: [PATCH 1/3] common: add apply_permutation file and function This algorithm is adapted from Raymond Chen's code: https://blogs.msdn.microsoft.com/oldnewthing/20170109-00/?p=95145 --- src/common/CMakeLists.txt | 1 + src/common/apply_permutation.h | 61 ++++++++++++++++++++++++++ tests/unit_tests/CMakeLists.txt | 1 + tests/unit_tests/apply_permutation.cpp | 45 +++++++++++++++++++ 4 files changed, 108 insertions(+) create mode 100644 src/common/apply_permutation.h create mode 100644 tests/unit_tests/apply_permutation.cpp diff --git a/src/common/CMakeLists.txt b/src/common/CMakeLists.txt index 55b8ad3e6..5adc9caf0 100644 --- a/src/common/CMakeLists.txt +++ b/src/common/CMakeLists.txt @@ -48,6 +48,7 @@ endif() set(common_headers) set(common_private_headers + apply_permutation.h base58.h boost_serialization_helper.h command_line.h diff --git a/src/common/apply_permutation.h b/src/common/apply_permutation.h new file mode 100644 index 000000000..4de224690 --- /dev/null +++ b/src/common/apply_permutation.h @@ -0,0 +1,61 @@ +// Copyright (c) 2017, 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. +// +// Most of this file is originally copyright (c) 2017 Raymond Chen, Microsoft +// This algorithm is adapted from Raymond Chen's code: +// https://blogs.msdn.microsoft.com/oldnewthing/20170109-00/?p=95145 + +#include +#include + +namespace tools +{ + +void apply_permutation(std::vector permutation, const std::function &swap) +{ + for (size_t i = 0; i < permutation.size(); ++i) + { + size_t current = i; + while (i != permutation[current]) + { + size_t next = permutation[current]; + swap(current, next); + permutation[current] = current; + current = next; + } + permutation[current] = current; + } +} + +template +void apply_permutation(const std::vector &permutation, std::vector &v) +{ + apply_permutation(permutation, [&v](size_t i0, size_t i1){ std::swap(v[i0], v[i1]); }); +} + +} diff --git a/tests/unit_tests/CMakeLists.txt b/tests/unit_tests/CMakeLists.txt index 2f62dc2aa..4286e0374 100644 --- a/tests/unit_tests/CMakeLists.txt +++ b/tests/unit_tests/CMakeLists.txt @@ -27,6 +27,7 @@ # THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. set(unit_tests_sources + apply_permutation.cpp address_from_url.cpp ban.cpp base58.cpp diff --git a/tests/unit_tests/apply_permutation.cpp b/tests/unit_tests/apply_permutation.cpp new file mode 100644 index 000000000..888a00746 --- /dev/null +++ b/tests/unit_tests/apply_permutation.cpp @@ -0,0 +1,45 @@ +// Copyright (c) 2017, 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. + +#include "gtest/gtest.h" +#include "common/apply_permutation.h" + +TEST(apply_permutation, empty) +{ + std::vector v = {}; + tools::apply_permutation({}, v); + ASSERT_EQ(v, std::vector({})); +} + +TEST(apply_permutation, reorder) +{ + // 0 1 2 3 4 5 6 + std::vector v = {8, 4, 6, 1, 7, 2, 4}; + tools::apply_permutation({3, 5, 6, 1, 2, 4, 0}, v); + ASSERT_EQ(v, std::vector({1, 2, 4, 4, 6, 7, 8})); +} From 16afab900dac52f2a08d93f8a9b73ed6aa6f0384 Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Tue, 12 Sep 2017 21:40:53 +0100 Subject: [PATCH 2/3] core: sort ins and outs key key image and public key, respectively This avoids leaking some small amount of information --- src/cryptonote_core/cryptonote_tx_utils.cpp | 36 ++++++++++++++++++--- src/cryptonote_core/cryptonote_tx_utils.h | 2 +- 2 files changed, 32 insertions(+), 6 deletions(-) diff --git a/src/cryptonote_core/cryptonote_tx_utils.cpp b/src/cryptonote_core/cryptonote_tx_utils.cpp index 94f069827..9b442029a 100644 --- a/src/cryptonote_core/cryptonote_tx_utils.cpp +++ b/src/cryptonote_core/cryptonote_tx_utils.cpp @@ -31,6 +31,7 @@ #include "include_base_utils.h" using namespace epee; +#include "common/apply_permutation.h" #include "cryptonote_tx_utils.h" #include "cryptonote_config.h" #include "cryptonote_basic/miner.h" @@ -156,7 +157,7 @@ namespace cryptonote return destinations[0].addr.m_view_public_key; } //--------------------------------------------------------------- - bool construct_tx_and_get_tx_key(const account_keys& sender_account_keys, const std::vector& sources, const std::vector& destinations, std::vector extra, transaction& tx, uint64_t unlock_time, crypto::secret_key &tx_key, bool rct) + bool construct_tx_and_get_tx_key(const account_keys& sender_account_keys, std::vector sources, const std::vector& destinations, std::vector extra, transaction& tx, uint64_t unlock_time, crypto::secret_key &tx_key, bool rct) { std::vector amount_keys; tx.set_null(); @@ -263,14 +264,25 @@ namespace cryptonote tx.vin.push_back(input_to_key); } - // "Shuffle" outs - std::vector shuffled_dsts(destinations); - std::random_shuffle(shuffled_dsts.begin(), shuffled_dsts.end(), [](unsigned int i) { return crypto::rand() % i; }); + // sort ins by their key image + std::vector ins_order(sources.size()); + for (size_t n = 0; n < sources.size(); ++n) + ins_order[n] = n; + std::sort(ins_order.begin(), ins_order.end(), [&](const size_t i0, const size_t i1) { + const txin_to_key &tk0 = boost::get(tx.vin[i0]); + const txin_to_key &tk1 = boost::get(tx.vin[i1]); + return memcmp(&tk0.k_image, &tk1.k_image, sizeof(tk0.k_image)) < 0; + }); + tools::apply_permutation(ins_order, [&] (size_t i0, size_t i1) { + std::swap(tx.vin[i0], tx.vin[i1]); + std::swap(in_contexts[i0], in_contexts[i1]); + std::swap(sources[i0], sources[i1]); + }); uint64_t summary_outs_money = 0; //fill outputs size_t output_index = 0; - for(const tx_destination_entry& dst_entr: shuffled_dsts) + for(const tx_destination_entry& dst_entr: destinations) { CHECK_AND_ASSERT_MES(dst_entr.amount > 0 || tx.version > 1, false, "Destination with wrong amount: " << dst_entr.amount); crypto::key_derivation derivation; @@ -297,6 +309,20 @@ namespace cryptonote summary_outs_money += dst_entr.amount; } + // sort outs by their public key + std::vector outs_order(tx.vout.size()); + for (size_t n = 0; n < tx.vout.size(); ++n) + outs_order[n] = n; + std::sort(outs_order.begin(), outs_order.end(), [&](size_t i0, size_t i1) { + const txout_to_key &tk0 = boost::get(tx.vout[i0].target); + const txout_to_key &tk1 = boost::get(tx.vout[i1].target); + return memcmp(&tk0.key, &tk1.key, sizeof(tk0.key)) < 0; + }); + tools::apply_permutation(outs_order, [&] (size_t i0, size_t i1) { + std::swap(tx.vout[i0], tx.vout[i1]); + std::swap(amount_keys[i0], amount_keys[i1]); + }); + //check money if(summary_outs_money > summary_inputs_money ) { diff --git a/src/cryptonote_core/cryptonote_tx_utils.h b/src/cryptonote_core/cryptonote_tx_utils.h index 7aa7c280d..69254fb5f 100644 --- a/src/cryptonote_core/cryptonote_tx_utils.h +++ b/src/cryptonote_core/cryptonote_tx_utils.h @@ -71,7 +71,7 @@ namespace cryptonote //--------------------------------------------------------------- crypto::public_key get_destination_view_key_pub(const std::vector &destinations, const account_keys &sender_keys); bool construct_tx(const account_keys& sender_account_keys, const std::vector& sources, const std::vector& destinations, std::vector extra, transaction& tx, uint64_t unlock_time); - bool construct_tx_and_get_tx_key(const account_keys& sender_account_keys, const std::vector& sources, const std::vector& destinations, std::vector extra, transaction& tx, uint64_t unlock_time, crypto::secret_key &tx_key, bool rct = false); + bool construct_tx_and_get_tx_key(const account_keys& sender_account_keys, std::vector sources, const std::vector& destinations, std::vector extra, transaction& tx, uint64_t unlock_time, crypto::secret_key &tx_key, bool rct = false); bool generate_genesis_block( block& bl From 6137a0b94d86e9f1c3321969da1c74f1d5e72b4f Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Tue, 12 Sep 2017 21:41:30 +0100 Subject: [PATCH 3/3] blockchain: reject unsorted ins and outs from v7 This ensures no information is leaked by the ordering --- src/common/apply_permutation.h | 9 +++++- src/cryptonote_core/blockchain.cpp | 39 ++++++++++++++++++++++++++ tests/unit_tests/apply_permutation.cpp | 29 +++++++++++++++++++ 3 files changed, 76 insertions(+), 1 deletion(-) diff --git a/src/common/apply_permutation.h b/src/common/apply_permutation.h index 4de224690..4fd952686 100644 --- a/src/common/apply_permutation.h +++ b/src/common/apply_permutation.h @@ -32,12 +32,18 @@ #include #include +#include "misc_log_ex.h" namespace tools { -void apply_permutation(std::vector permutation, const std::function &swap) +template +void apply_permutation(std::vector permutation, const F &swap) { + //sanity check + for (size_t n = 0; n < permutation.size(); ++n) + CHECK_AND_ASSERT_THROW_MES(std::find(permutation.begin(), permutation.end(), n) != permutation.end(), "Bad permutation"); + for (size_t i = 0; i < permutation.size(); ++i) { size_t current = i; @@ -55,6 +61,7 @@ void apply_permutation(std::vector permutation, const std::function void apply_permutation(const std::vector &permutation, std::vector &v) { + CHECK_AND_ASSERT_THROW_MES(permutation.size() == v.size(), "Mismatched vector sizes"); apply_permutation(permutation, [&v](size_t i0, size_t i1){ std::swap(v[i0], v[i1]); }); } diff --git a/src/cryptonote_core/blockchain.cpp b/src/cryptonote_core/blockchain.cpp index 93a4e26f8..c0d142979 100644 --- a/src/cryptonote_core/blockchain.cpp +++ b/src/cryptonote_core/blockchain.cpp @@ -2333,6 +2333,26 @@ bool Blockchain::check_tx_outputs(const transaction& tx, tx_verification_context } } + // from v7, sorted outs + if (m_hardfork->get_current_version() >= 7) { + const crypto::public_key *last_key = NULL; + for (size_t n = 0; n < tx.vout.size(); ++n) + { + const tx_out &o = tx.vout[n]; + if (o.target.type() == typeid(txout_to_key)) + { + const txout_to_key& out_to_key = boost::get(o.target); + if (last_key && memcmp(&out_to_key.key, last_key, sizeof(*last_key)) >= 0) + { + MERROR_VER("transaction has unsorted outputs"); + tvc.m_invalid_output = true; + return false; + } + last_key = &out_to_key.key; + } + } + } + return true; } //------------------------------------------------------------------ @@ -2501,6 +2521,25 @@ bool Blockchain::check_tx_inputs(transaction& tx, tx_verification_context &tvc, } } + // from v7, sorted ins + if (hf_version >= 7) { + const crypto::key_image *last_key_image = NULL; + for (size_t n = 0; n < tx.vin.size(); ++n) + { + const txin_v &txin = tx.vin[n]; + if (txin.type() == typeid(txin_to_key)) + { + const txin_to_key& in_to_key = boost::get(txin); + if (last_key_image && memcmp(&in_to_key.k_image, last_key_image, sizeof(*last_key_image)) >= 0) + { + MERROR_VER("transaction has unsorted inputs"); + tvc.m_verifivation_failed = true; + return false; + } + last_key_image = &in_to_key.k_image; + } + } + } auto it = m_check_txin_table.find(tx_prefix_hash); if(it == m_check_txin_table.end()) { diff --git a/tests/unit_tests/apply_permutation.cpp b/tests/unit_tests/apply_permutation.cpp index 888a00746..a008b74ee 100644 --- a/tests/unit_tests/apply_permutation.cpp +++ b/tests/unit_tests/apply_permutation.cpp @@ -43,3 +43,32 @@ TEST(apply_permutation, reorder) tools::apply_permutation({3, 5, 6, 1, 2, 4, 0}, v); ASSERT_EQ(v, std::vector({1, 2, 4, 4, 6, 7, 8})); } + +TEST(apply_permutation, bad_size) +{ + std::vector v_large = {8, 4, 6, 1, 7, 2, 4, 9}; + std::vector v_small = {8, 4, 6, 1, 7, 2}; + try + { + tools::apply_permutation({3, 5, 6, 1, 2, 4, 0}, v_large); + ASSERT_FALSE(true); + } + catch (const std::exception &e) {} + try + { + tools::apply_permutation({3, 5, 6, 1, 2, 4, 0}, v_small); + ASSERT_FALSE(true); + } + catch (const std::exception &e) {} +} + +TEST(apply_permutation, bad_permutation) +{ + std::vector v = {8, 4, 6, 1, 7, 2, 4}; + try + { + tools::apply_permutation({3, 5, 6, 1, 2, 4, 1}, v); + ASSERT_FALSE(true); + } + catch (const std::exception &e) {} +}