From 4ef1ab3e385f76c053b91f3a502ffcc20887c5ca Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Sat, 16 May 2020 18:12:55 +0000 Subject: [PATCH 01/17] epee: use memwipe rather than memset for md5 secrets That's used by HTTP auth now --- contrib/epee/include/md5_l.inl | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/contrib/epee/include/md5_l.inl b/contrib/epee/include/md5_l.inl index 8e339e006..cb2bd54f9 100644 --- a/contrib/epee/include/md5_l.inl +++ b/contrib/epee/include/md5_l.inl @@ -277,7 +277,7 @@ namespace md5 /* Zeroize sensitive information. */ - MD5_memset ((POINTER)context, 0, sizeof (*context)); + memwipe ((POINTER)context, sizeof (*context)); } /* MD5 basic transformation. Transforms state based on block. @@ -369,7 +369,7 @@ namespace md5 /* Zeroize sensitive information. */ - MD5_memset ((POINTER)x, 0, sizeof (x)); + memwipe ((POINTER)x, sizeof (x)); } /* Note: Replace "for loop" with standard memcpy if possible. @@ -431,9 +431,9 @@ namespace md5 MD5Update(&hmac->octx, k_opad, 64); /* apply outer pad */ /* scrub the pads and key context (if used) */ - MD5_memset( (POINTER)&k_ipad, 0, sizeof(k_ipad)); - MD5_memset( (POINTER)&k_opad, 0, sizeof(k_opad)); - MD5_memset( (POINTER)&tk, 0, sizeof(tk)); + memwipe( (POINTER)&k_ipad, sizeof(k_ipad)); + memwipe( (POINTER)&k_opad, sizeof(k_opad)); + memwipe( (POINTER)&tk, sizeof(tk)); /* and we're done. */ } @@ -459,7 +459,7 @@ namespace md5 state->istate[lupe] = htonl(hmac.ictx.state[lupe]); state->ostate[lupe] = htonl(hmac.octx.state[lupe]); } - MD5_memset( (POINTER)&hmac, 0, sizeof(hmac)); + memwipe( (POINTER)&hmac, sizeof(hmac)); } From 60cf2372104ebdc9590b55adf89543f5491894c6 Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Sat, 16 May 2020 19:54:55 +0000 Subject: [PATCH 02/17] protocol: don't drop a connection if we can't get a compatible chain This can now happen if: - we have a pruned db - we have not connected to the monero network for a while - we connect to a node - that node asks us for history - we only have a pruned version of the most recent common block In that case, it's better to not reply but keep the connection alive, so we can sync off it. --- src/cryptonote_protocol/cryptonote_protocol_handler.inl | 1 - 1 file changed, 1 deletion(-) diff --git a/src/cryptonote_protocol/cryptonote_protocol_handler.inl b/src/cryptonote_protocol/cryptonote_protocol_handler.inl index 773269022..b1ca36225 100644 --- a/src/cryptonote_protocol/cryptonote_protocol_handler.inl +++ b/src/cryptonote_protocol/cryptonote_protocol_handler.inl @@ -1721,7 +1721,6 @@ skip: if(!m_core.find_blockchain_supplement(arg.block_ids, !arg.prune, r)) { LOG_ERROR_CCONTEXT("Failed to handle NOTIFY_REQUEST_CHAIN."); - drop_connection(context, false, false); return 1; } MLOG_P2P_MESSAGE("-->>NOTIFY_RESPONSE_CHAIN_ENTRY: m_start_height=" << r.start_height << ", m_total_height=" << r.total_height << ", m_block_ids.size()=" << r.m_block_ids.size()); From 009abd48e59e84862ff0b1076a9c7458ceee59ea Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Sat, 16 May 2020 20:11:40 +0000 Subject: [PATCH 03/17] blockchain: detect and log bad difficulty calculations --- src/cryptonote_core/blockchain.cpp | 77 +++++++++++++++++++++++++++--- 1 file changed, 71 insertions(+), 6 deletions(-) diff --git a/src/cryptonote_core/blockchain.cpp b/src/cryptonote_core/blockchain.cpp index cc6f49fbe..830d45259 100644 --- a/src/cryptonote_core/blockchain.cpp +++ b/src/cryptonote_core/blockchain.cpp @@ -812,12 +812,20 @@ bool Blockchain::get_block_by_hash(const crypto::hash &h, block &blk, bool *orph // less blocks than desired if there aren't enough. difficulty_type Blockchain::get_difficulty_for_next_block() { + LOG_PRINT_L3("Blockchain::" << __func__); + + std::stringstream ss; + bool print = false; + + int done = 0; + ss << "get_difficulty_for_next_block: height " << m_db->height() << std::endl; if (m_fixed_difficulty) { return m_db->height() ? m_fixed_difficulty : 1; } - LOG_PRINT_L3("Blockchain::" << __func__); +start: + difficulty_type D = 0; crypto::hash top_hash = get_tail_id(); { @@ -826,25 +834,32 @@ difficulty_type Blockchain::get_difficulty_for_next_block() // something a bit out of date, but that's fine since anything which // requires the blockchain lock will have acquired it in the first place, // and it will be unlocked only when called from the getinfo RPC + ss << "Locked, tail id " << top_hash << ", cached is " << m_difficulty_for_next_block_top_hash << std::endl; if (top_hash == m_difficulty_for_next_block_top_hash) - return m_difficulty_for_next_block; + { + ss << "Same, using cached diff " << m_difficulty_for_next_block << std::endl; + D = m_difficulty_for_next_block; + } } CRITICAL_REGION_LOCAL(m_blockchain_lock); std::vector timestamps; std::vector difficulties; uint64_t height; - top_hash = get_tail_id(height); // get it again now that we have the lock - ++height; // top block height to blockchain height - + auto new_top_hash = get_tail_id(height); // get it again now that we have the lock + ++height; uint8_t version = get_current_hard_fork_version(); uint64_t difficulty_blocks_count = version >= 11 ? DIFFICULTY_BLOCKS_COUNT_V3 : version <= 10 && version >= 8 ? DIFFICULTY_BLOCKS_COUNT_V2 : DIFFICULTY_BLOCKS_COUNT; + if (!(new_top_hash == top_hash)) D=0; + ss << "Re-locked, height " << height << ", tail id " << new_top_hash << (new_top_hash == top_hash ? "" : " (different)") << std::endl; + top_hash = new_top_hash; // ND: Speedup // 1. Keep a list of the last 735 (or less) blocks that is used to compute difficulty, // then when the next block difficulty is queried, push the latest height data and // pop the oldest one from the list. This only requires 1x read per height instead // of doing 735 (DIFFICULTY_BLOCKS_COUNT). + bool check = false; if (m_timestamps_and_difficulties_height != 0 && ((height - m_timestamps_and_difficulties_height) == 1) && m_timestamps.size() >= difficulty_blocks_count) { uint64_t index = height - 1; @@ -859,8 +874,12 @@ difficulty_type Blockchain::get_difficulty_for_next_block() m_timestamps_and_difficulties_height = height; timestamps = m_timestamps; difficulties = m_difficulties; + check = true; } - else + //else + std::vector timestamps_from_cache = timestamps; + std::vector difficulties_from_cache = difficulties; + { uint64_t offset = height - std::min (height, static_cast(difficulty_blocks_count)); if (offset == 0) @@ -873,16 +892,40 @@ difficulty_type Blockchain::get_difficulty_for_next_block() timestamps.reserve(height - offset); difficulties.reserve(height - offset); } + ss << "Looking up " << (height - offset) << " from " << offset << std::endl; for (; offset < height; offset++) { timestamps.push_back(m_db->get_block_timestamp(offset)); difficulties.push_back(m_db->get_block_cumulative_difficulty(offset)); } + if (check) if (timestamps != timestamps_from_cache || difficulties !=difficulties_from_cache) + { + ss << "Inconsistency XXX:" << std::endl; + ss << "top hash: "<height(); + uint64_t sh = dbh < 10000 ? 0 : dbh - 10000; + ss << "History from -10k at :" << dbh << ", from " << sh << std::endl; + for (uint64_t h = sh; h < dbh; ++h) + { + uint64_t ts = m_db->get_block_timestamp(h); + difficulty_type d = m_db->get_block_cumulative_difficulty(h); + ss << " " << h << " " << ts << " " << d << std::endl; + } + print = true; + } m_timestamps_and_difficulties_height = height; m_timestamps = timestamps; m_difficulties = difficulties; } + size_t target = get_difficulty_target(); uint64_t T = DIFFICULTY_TARGET_V2; uint64_t N = DIFFICULTY_WINDOW_V3; @@ -911,6 +954,28 @@ difficulty_type Blockchain::get_difficulty_for_next_block() CRITICAL_REGION_LOCAL1(m_difficulty_lock); m_difficulty_for_next_block_top_hash = top_hash; m_difficulty_for_next_block = diff; + if (D && D != diff) + { + ss << "XXX Mismatch at " << height << "/" << top_hash << "/" << get_tail_id() << ": cached " << D << ", real " << diff << std::endl; + print = true; + } + + ++done; + if (done == 1 && D && D != diff) + { + print = true; + ss << "Might be a race. Let's see what happens if we try again..." << std::endl; + epee::misc_utils::sleep_no_w(100); + goto start; + } + ss << "Diff for " << top_hash << ": " << diff << std::endl; + if (print) + { + MGINFO("START DUMP"); + MGINFO(ss.str()); + MGINFO("END DUMP"); + MGINFO("Please send moneromooo on Freenode the contents of this log, from a couple dozen lines before START DUMP to END DUMP"); + } return diff; } //------------------------------------------------------------------ From 1125b332d221e583a68d5fcffcf3461609d91d59 Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Tue, 28 Apr 2020 13:28:55 +0000 Subject: [PATCH 04/17] easylogging++: sanitize log payload Some of it might be coming from untrusted sources Reported by itsunixiknowthis --- external/easylogging++/easylogging++.cc | 96 ++++++++++++++++++++ src/common/CMakeLists.txt | 3 +- src/common/utf8.h | 114 ++++++++++++++++++++++++ src/mnemonics/language_base.h | 74 +-------------- 4 files changed, 216 insertions(+), 71 deletions(-) create mode 100644 src/common/utf8.h diff --git a/external/easylogging++/easylogging++.cc b/external/easylogging++/easylogging++.cc index 8439bec0b..0d748c225 100644 --- a/external/easylogging++/easylogging++.cc +++ b/external/easylogging++/easylogging++.cc @@ -2475,6 +2475,100 @@ void DefaultLogDispatchCallback::handle(const LogDispatchData* data) { } } + +template +static inline std::string utf8canonical(const std::string &s, Transform t = [](wint_t c)->wint_t { return c; }) +{ + std::string sc = ""; + size_t avail = s.size(); + const char *ptr = s.data(); + wint_t cp = 0; + int bytes = 1; + char wbuf[8], *wptr; + while (avail--) + { + if ((*ptr & 0x80) == 0) + { + cp = *ptr++; + bytes = 1; + } + else if ((*ptr & 0xe0) == 0xc0) + { + if (avail < 1) + throw std::runtime_error("Invalid UTF-8"); + cp = (*ptr++ & 0x1f) << 6; + cp |= *ptr++ & 0x3f; + --avail; + bytes = 2; + } + else if ((*ptr & 0xf0) == 0xe0) + { + if (avail < 2) + throw std::runtime_error("Invalid UTF-8"); + cp = (*ptr++ & 0xf) << 12; + cp |= (*ptr++ & 0x3f) << 6; + cp |= *ptr++ & 0x3f; + avail -= 2; + bytes = 3; + } + else if ((*ptr & 0xf8) == 0xf0) + { + if (avail < 3) + throw std::runtime_error("Invalid UTF-8"); + cp = (*ptr++ & 0x7) << 18; + cp |= (*ptr++ & 0x3f) << 12; + cp |= (*ptr++ & 0x3f) << 6; + cp |= *ptr++ & 0x3f; + avail -= 3; + bytes = 4; + } + else + throw std::runtime_error("Invalid UTF-8"); + + cp = t(cp); + if (cp <= 0x7f) + bytes = 1; + else if (cp <= 0x7ff) + bytes = 2; + else if (cp <= 0xffff) + bytes = 3; + else if (cp <= 0x10ffff) + bytes = 4; + else + throw std::runtime_error("Invalid code point UTF-8 transformation"); + + wptr = wbuf; + switch (bytes) + { + case 1: *wptr++ = cp; break; + case 2: *wptr++ = 0xc0 | (cp >> 6); *wptr++ = 0x80 | (cp & 0x3f); break; + case 3: *wptr++ = 0xe0 | (cp >> 12); *wptr++ = 0x80 | ((cp >> 6) & 0x3f); *wptr++ = 0x80 | (cp & 0x3f); break; + case 4: *wptr++ = 0xf0 | (cp >> 18); *wptr++ = 0x80 | ((cp >> 12) & 0x3f); *wptr++ = 0x80 | ((cp >> 6) & 0x3f); *wptr++ = 0x80 | (cp & 0x3f); break; + default: throw std::runtime_error("Invalid UTF-8"); + } + *wptr = 0; + sc.append(wbuf, bytes); + cp = 0; + bytes = 1; + } + return sc; +} + +void sanitize(std::string &s) +{ + s = utf8canonical(s, [](wint_t c)->wint_t { + if (c == 9 || c == 10 || c == 13) + return c; + if (c < 0x20) + return '?'; + if (c == 0x7f) + return '?'; + if (c >= 0x80 && c <= 0x9f) + return '?'; + return c; + }); +} + void DefaultLogDispatchCallback::dispatch(base::type::string_t&& rawLinePrefix, base::type::string_t&& rawLinePayload, base::type::string_t&& logLine) { if (m_data->dispatchAction() == base::DispatchAction::NormalLog || m_data->dispatchAction() == base::DispatchAction::FileOnlyLog) { if (m_data->logMessage()->logger()->m_typedConfigurations->toFile(m_data->logMessage()->level())) { @@ -2506,6 +2600,8 @@ void DefaultLogDispatchCallback::dispatch(base::type::string_t&& rawLinePrefix, m_data->logMessage()->logger()->logBuilder()->setColor(el::base::utils::colorFromLevel(level), false); ELPP_COUT << rawLinePrefix; m_data->logMessage()->logger()->logBuilder()->setColor(color == el::Color::Default ? el::base::utils::colorFromLevel(level): color, color != el::Color::Default); + try { sanitize(rawLinePayload); } + catch (const std::exception &e) { rawLinePayload = ""; } ELPP_COUT << rawLinePayload; m_data->logMessage()->logger()->logBuilder()->setColor(el::Color::Default, false); ELPP_COUT << std::flush; diff --git a/src/common/CMakeLists.txt b/src/common/CMakeLists.txt index f06737b31..35b3555a2 100644 --- a/src/common/CMakeLists.txt +++ b/src/common/CMakeLists.txt @@ -86,7 +86,8 @@ set(common_private_headers updates.h aligned.h timings.h - combinator.h) + combinator.h + utf8.h) monero_private_headers(common ${common_private_headers}) diff --git a/src/common/utf8.h b/src/common/utf8.h new file mode 100644 index 000000000..60247f1b2 --- /dev/null +++ b/src/common/utf8.h @@ -0,0 +1,114 @@ +// Copyright (c) 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. + +#pragma once + +#include +#include +#include + +namespace tools +{ + template + inline T utf8canonical(const T &s, Transform t = [](wint_t c)->wint_t { return c; }) + { + T sc = ""; + size_t avail = s.size(); + const char *ptr = s.data(); + wint_t cp = 0; + int bytes = 1; + char wbuf[8], *wptr; + while (avail--) + { + if ((*ptr & 0x80) == 0) + { + cp = *ptr++; + bytes = 1; + } + else if ((*ptr & 0xe0) == 0xc0) + { + if (avail < 1) + throw std::runtime_error("Invalid UTF-8"); + cp = (*ptr++ & 0x1f) << 6; + cp |= *ptr++ & 0x3f; + --avail; + bytes = 2; + } + else if ((*ptr & 0xf0) == 0xe0) + { + if (avail < 2) + throw std::runtime_error("Invalid UTF-8"); + cp = (*ptr++ & 0xf) << 12; + cp |= (*ptr++ & 0x3f) << 6; + cp |= *ptr++ & 0x3f; + avail -= 2; + bytes = 3; + } + else if ((*ptr & 0xf8) == 0xf0) + { + if (avail < 3) + throw std::runtime_error("Invalid UTF-8"); + cp = (*ptr++ & 0x7) << 18; + cp |= (*ptr++ & 0x3f) << 12; + cp |= (*ptr++ & 0x3f) << 6; + cp |= *ptr++ & 0x3f; + avail -= 3; + bytes = 4; + } + else + throw std::runtime_error("Invalid UTF-8"); + + cp = t(cp); + if (cp <= 0x7f) + bytes = 1; + else if (cp <= 0x7ff) + bytes = 2; + else if (cp <= 0xffff) + bytes = 3; + else if (cp <= 0x10ffff) + bytes = 4; + else + throw std::runtime_error("Invalid code point UTF-8 transformation"); + + wptr = wbuf; + switch (bytes) + { + case 1: *wptr++ = cp; break; + case 2: *wptr++ = 0xc0 | (cp >> 6); *wptr++ = 0x80 | (cp & 0x3f); break; + case 3: *wptr++ = 0xe0 | (cp >> 12); *wptr++ = 0x80 | ((cp >> 6) & 0x3f); *wptr++ = 0x80 | (cp & 0x3f); break; + case 4: *wptr++ = 0xf0 | (cp >> 18); *wptr++ = 0x80 | ((cp >> 12) & 0x3f); *wptr++ = 0x80 | ((cp >> 6) & 0x3f); *wptr++ = 0x80 | (cp & 0x3f); break; + default: throw std::runtime_error("Invalid UTF-8"); + } + *wptr = 0; + sc.append(wbuf, bytes); + cp = 0; + bytes = 1; + } + return sc; + } +} diff --git a/src/mnemonics/language_base.h b/src/mnemonics/language_base.h index 7d2599e9a..ad09dc5fa 100644 --- a/src/mnemonics/language_base.h +++ b/src/mnemonics/language_base.h @@ -41,6 +41,7 @@ #include #include "misc_log_ex.h" #include "fnv1.h" +#include "common/utf8.h" /*! * \namespace Language @@ -73,78 +74,11 @@ namespace Language return prefix; } - template - inline T utf8canonical(const T &s) - { - T sc = ""; - size_t avail = s.size(); - const char *ptr = s.data(); - wint_t cp = 0; - int bytes = 1; - char wbuf[8], *wptr; - while (avail--) - { - if ((*ptr & 0x80) == 0) - { - cp = *ptr++; - bytes = 1; - } - else if ((*ptr & 0xe0) == 0xc0) - { - if (avail < 1) - throw std::runtime_error("Invalid UTF-8"); - cp = (*ptr++ & 0x1f) << 6; - cp |= *ptr++ & 0x3f; - --avail; - bytes = 2; - } - else if ((*ptr & 0xf0) == 0xe0) - { - if (avail < 2) - throw std::runtime_error("Invalid UTF-8"); - cp = (*ptr++ & 0xf) << 12; - cp |= (*ptr++ & 0x3f) << 6; - cp |= *ptr++ & 0x3f; - avail -= 2; - bytes = 3; - } - else if ((*ptr & 0xf8) == 0xf0) - { - if (avail < 3) - throw std::runtime_error("Invalid UTF-8"); - cp = (*ptr++ & 0x7) << 18; - cp |= (*ptr++ & 0x3f) << 12; - cp |= (*ptr++ & 0x3f) << 6; - cp |= *ptr++ & 0x3f; - avail -= 3; - bytes = 4; - } - else - throw std::runtime_error("Invalid UTF-8"); - - cp = std::towlower(cp); - wptr = wbuf; - switch (bytes) - { - case 1: *wptr++ = cp; break; - case 2: *wptr++ = 0xc0 | (cp >> 6); *wptr++ = 0x80 | (cp & 0x3f); break; - case 3: *wptr++ = 0xe0 | (cp >> 12); *wptr++ = 0x80 | ((cp >> 6) & 0x3f); *wptr++ = 0x80 | (cp & 0x3f); break; - case 4: *wptr++ = 0xf0 | (cp >> 18); *wptr++ = 0x80 | ((cp >> 12) & 0x3f); *wptr++ = 0x80 | ((cp >> 6) & 0x3f); *wptr++ = 0x80 | (cp & 0x3f); break; - default: throw std::runtime_error("Invalid UTF-8"); - } - *wptr = 0; - sc += T(wbuf, bytes); - cp = 0; - bytes = 1; - } - return sc; - } - struct WordHash { std::size_t operator()(const epee::wipeable_string &s) const { - const epee::wipeable_string sc = utf8canonical(s); + const epee::wipeable_string sc = tools::utf8canonical(s, [](wint_t c) -> wint_t { return std::towlower(c); }); return epee::fnv::FNV1a(sc.data(), sc.size()); } }; @@ -153,8 +87,8 @@ namespace Language { bool operator()(const epee::wipeable_string &s0, const epee::wipeable_string &s1) const { - const epee::wipeable_string s0c = utf8canonical(s0); - const epee::wipeable_string s1c = utf8canonical(s1); + const epee::wipeable_string s0c = tools::utf8canonical(s0, [](wint_t c) -> wint_t { return std::towlower(c); }); + const epee::wipeable_string s1c = tools::utf8canonical(s1, [](wint_t c) -> wint_t { return std::towlower(c); }); return s0c == s1c; } }; From 24660441a9f921f72913a59d70af7eb23df6ce32 Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Sun, 17 May 2020 13:48:40 +0000 Subject: [PATCH 05/17] cryptonote_protocol: reject requests/notifications before handshake Reported by xnbya --- .../cryptonote_protocol_handler.inl | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/src/cryptonote_protocol/cryptonote_protocol_handler.inl b/src/cryptonote_protocol/cryptonote_protocol_handler.inl index b1ca36225..c8359dc11 100644 --- a/src/cryptonote_protocol/cryptonote_protocol_handler.inl +++ b/src/cryptonote_protocol/cryptonote_protocol_handler.inl @@ -793,6 +793,12 @@ namespace cryptonote int t_cryptonote_protocol_handler::handle_request_fluffy_missing_tx(int command, NOTIFY_REQUEST_FLUFFY_MISSING_TX::request& arg, cryptonote_connection_context& context) { MLOG_P2P_MESSAGE("Received NOTIFY_REQUEST_FLUFFY_MISSING_TX (" << arg.missing_tx_indices.size() << " txes), block hash " << arg.block_hash); + if (context.m_state == cryptonote_connection_context::state_before_handshake) + { + LOG_ERROR_CCONTEXT("Requested fluffy tx before handshake, dropping connection"); + drop_connection(context, false, false); + return 1; + } std::vector> local_blocks; std::vector local_txs; @@ -884,6 +890,8 @@ namespace cryptonote int t_cryptonote_protocol_handler::handle_notify_get_txpool_complement(int command, NOTIFY_GET_TXPOOL_COMPLEMENT::request& arg, cryptonote_connection_context& context) { MLOG_P2P_MESSAGE("Received NOTIFY_GET_TXPOOL_COMPLEMENT (" << arg.hashes.size() << " txes)"); + if(context.m_state != cryptonote_connection_context::state_normal) + return 1; std::vector> local_blocks; std::vector local_txs; @@ -987,6 +995,12 @@ namespace cryptonote template int t_cryptonote_protocol_handler::handle_request_get_objects(int command, NOTIFY_REQUEST_GET_OBJECTS::request& arg, cryptonote_connection_context& context) { + if (context.m_state == cryptonote_connection_context::state_before_handshake) + { + LOG_ERROR_CCONTEXT("Requested objects before handshake, dropping connection"); + drop_connection(context, false, false); + return 1; + } MLOG_P2P_MESSAGE("Received NOTIFY_REQUEST_GET_OBJECTS (" << arg.blocks.size() << " blocks)"); if (arg.blocks.size() > CURRENCY_PROTOCOL_MAX_OBJECT_REQUEST_COUNT) { @@ -1717,6 +1731,12 @@ skip: int t_cryptonote_protocol_handler::handle_request_chain(int command, NOTIFY_REQUEST_CHAIN::request& arg, cryptonote_connection_context& context) { MLOG_P2P_MESSAGE("Received NOTIFY_REQUEST_CHAIN (" << arg.block_ids.size() << " blocks"); + if (context.m_state == cryptonote_connection_context::state_before_handshake) + { + LOG_ERROR_CCONTEXT("Requested chain before handshake, dropping connection"); + drop_connection(context, false, false); + return 1; + } NOTIFY_RESPONSE_CHAIN_ENTRY::request r; if(!m_core.find_blockchain_supplement(arg.block_ids, !arg.prune, r)) { From e062e1efe349e18067325e428d02eb4a3e34fe9e Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Sun, 17 May 2020 14:01:28 +0000 Subject: [PATCH 06/17] cryptonote_protocol: stricter limit to number of objects requested Reported by xnbya --- src/cryptonote_protocol/cryptonote_protocol_handler.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/cryptonote_protocol/cryptonote_protocol_handler.h b/src/cryptonote_protocol/cryptonote_protocol_handler.h index e2ad3727f..3055474ef 100644 --- a/src/cryptonote_protocol/cryptonote_protocol_handler.h +++ b/src/cryptonote_protocol/cryptonote_protocol_handler.h @@ -51,7 +51,8 @@ PUSH_WARNINGS DISABLE_VS_WARNINGS(4355) #define LOCALHOST_INT 2130706433 -#define CURRENCY_PROTOCOL_MAX_OBJECT_REQUEST_COUNT 500 +#define CURRENCY_PROTOCOL_MAX_OBJECT_REQUEST_COUNT 100 +static_assert(CURRENCY_PROTOCOL_MAX_OBJECT_REQUEST_COUNT >= BLOCKS_SYNCHRONIZING_DEFAULT_COUNT_PRE_V4, "Invalid CURRENCY_PROTOCOL_MAX_OBJECT_REQUEST_COUNT"); namespace cryptonote { From 4eb2f5c2c8a203dd35644b028cf29bbd1b793f2a Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Mon, 18 May 2020 14:16:38 +0000 Subject: [PATCH 07/17] serialization: fix bad rapidjson api usage --- src/serialization/json_object.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/serialization/json_object.cpp b/src/serialization/json_object.cpp index 6228b4bec..5c042aa7b 100644 --- a/src/serialization/json_object.cpp +++ b/src/serialization/json_object.cpp @@ -120,7 +120,7 @@ void read_hex(const rapidjson::Value& val, epee::span dest) throw WRONG_TYPE("string"); } - if (!epee::from_hex::to_buffer(dest, {val.GetString(), val.Size()})) + if (!epee::from_hex::to_buffer(dest, {val.GetString(), val.GetStringLength()})) { throw BAD_INPUT(); } From 18cb1753b768ebf00802f03ad557fc2afe4c40da Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Tue, 19 May 2020 10:45:40 +0000 Subject: [PATCH 08/17] wallet2: fix multisig data clearing stomping on a vector --- src/wallet/wallet2.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/wallet/wallet2.cpp b/src/wallet/wallet2.cpp index 4779c3335..9ede4c9cd 100644 --- a/src/wallet/wallet2.cpp +++ b/src/wallet/wallet2.cpp @@ -13083,7 +13083,7 @@ size_t wallet2::import_multisig(std::vector blobs) CHECK_AND_ASSERT_THROW_MES(info.size() + 1 <= m_multisig_signers.size() && info.size() + 1 >= m_multisig_threshold, "Wrong number of multisig sources"); std::vector> k; - auto wiper = epee::misc_utils::create_scope_leave_handler([&](){memwipe(k.data(), k.size() * sizeof(k[0]));}); + auto wiper = epee::misc_utils::create_scope_leave_handler([&](){for (auto &v: k) memwipe(v.data(), v.size() * sizeof(v[0]));}); k.reserve(m_transfers.size()); for (const auto &td: m_transfers) k.push_back(td.m_multisig_k); From 9c102292688ca61091d5db6ef56f80900fbb0087 Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Tue, 19 May 2020 16:13:49 +0000 Subject: [PATCH 09/17] protocol: move the "peer claims higher version" warning to debug Because there's a neverending supply of cunts claiming a wrong version just to say "look at me" I guess --- src/cryptonote_protocol/cryptonote_protocol_handler.inl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/cryptonote_protocol/cryptonote_protocol_handler.inl b/src/cryptonote_protocol/cryptonote_protocol_handler.inl index c8359dc11..2275fd2a8 100644 --- a/src/cryptonote_protocol/cryptonote_protocol_handler.inl +++ b/src/cryptonote_protocol/cryptonote_protocol_handler.inl @@ -308,9 +308,9 @@ namespace cryptonote if (version >= 6 && version != hshd.top_version) { if (version < hshd.top_version && version == m_core.get_ideal_hard_fork_version()) - MCLOG_RED(el::Level::Warning, "global", context << " peer claims higher version than we think (" << + MDEBUG(context << " peer claims higher version than we think (" << (unsigned)hshd.top_version << " for " << (hshd.current_height - 1) << " instead of " << (unsigned)version << - ") - we may be forked from the network and a software upgrade may be needed"); + ") - we may be forked from the network and a software upgrade may be needed, or that peer is broken or malicious"); return false; } } From 19f284ddf9f3ea2ab6532a39ec8b63a24de4cdce Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Tue, 19 May 2020 16:23:59 +0000 Subject: [PATCH 10/17] cryptonote_core: remove "We are most likely forked" message It's time based and we don't have forks every 6 months anymore --- src/cryptonote_core/cryptonote_core.cpp | 24 ------------------------ src/cryptonote_core/cryptonote_core.h | 12 ------------ 2 files changed, 36 deletions(-) diff --git a/src/cryptonote_core/cryptonote_core.cpp b/src/cryptonote_core/cryptonote_core.cpp index 48b776b7a..f4c51078b 100644 --- a/src/cryptonote_core/cryptonote_core.cpp +++ b/src/cryptonote_core/cryptonote_core.cpp @@ -1721,7 +1721,6 @@ namespace cryptonote m_starter_message_showed = true; } - m_fork_moaner.do_call(boost::bind(&core::check_fork_time, this)); m_txpool_auto_relayer.do_call(boost::bind(&core::relay_txpool_transactions, this)); m_check_updates_interval.do_call(boost::bind(&core::check_updates, this)); m_check_disk_space_interval.do_call(boost::bind(&core::check_disk_space, this)); @@ -1732,29 +1731,6 @@ namespace cryptonote return true; } //----------------------------------------------------------------------------------------------- - bool core::check_fork_time() - { - if (m_nettype == FAKECHAIN) - return true; - - HardFork::State state = m_blockchain_storage.get_hard_fork_state(); - el::Level level; - switch (state) { - case HardFork::LikelyForked: - level = el::Level::Warning; - MCLOG_RED(level, "global", "**********************************************************************"); - MCLOG_RED(level, "global", "Last scheduled hard fork is too far in the past."); - MCLOG_RED(level, "global", "We are most likely forked from the network. Daemon update needed now."); - MCLOG_RED(level, "global", "**********************************************************************"); - break; - case HardFork::UpdateNeeded: - break; - default: - break; - } - return true; - } - //----------------------------------------------------------------------------------------------- uint8_t core::get_ideal_hard_fork_version() const { return get_blockchain_storage().get_ideal_hard_fork_version(); diff --git a/src/cryptonote_core/cryptonote_core.h b/src/cryptonote_core/cryptonote_core.h index 988f25ceb..68c5651f1 100644 --- a/src/cryptonote_core/cryptonote_core.h +++ b/src/cryptonote_core/cryptonote_core.h @@ -1000,18 +1000,6 @@ namespace cryptonote */ bool check_tx_inputs_keyimages_domain(const transaction& tx) const; - /** - * @brief checks HardFork status and prints messages about it - * - * Checks the status of HardFork and logs/prints if an update to - * the daemon is necessary. - * - * @note see Blockchain::get_hard_fork_state and HardFork::State - * - * @return true - */ - bool check_fork_time(); - /** * @brief attempts to relay any transactions in the mempool which need it * From e0a963355715acfba797056e7b916d6492c6e41a Mon Sep 17 00:00:00 2001 From: Doyle Date: Tue, 19 May 2020 18:45:32 +1000 Subject: [PATCH 11/17] ByteSlice: Fix persisting ptr to std::moved SSO buffer The Bug: 1. Construct `byte_slice.portion_` with `epee::span(buffer)` which copies a pointer to the SSO buffer to `byte_slice.portion_` 2. It constructs `byte_slice.storage_` with `std::move(buffer)` (normally this swap pointers, but SSO means a memcpy and clear on the original SSO buffer) 3. `slice.data()` returns a pointer from `slice.portion_` that points to the original SSO cleared buffer, `slice.storage_` has the actual string. --- contrib/epee/src/byte_slice.cpp | 5 ++++- tests/unit_tests/epee_utils.cpp | 23 +++++++++++++++++++++++ 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/contrib/epee/src/byte_slice.cpp b/contrib/epee/src/byte_slice.cpp index 12cc83e6c..faf7689be 100644 --- a/contrib/epee/src/byte_slice.cpp +++ b/contrib/epee/src/byte_slice.cpp @@ -133,10 +133,13 @@ namespace epee template byte_slice::byte_slice(const adapt_buffer, T&& buffer) - : storage_(nullptr), portion_(to_byte_span(to_span(buffer))) + : storage_(nullptr), portion_(nullptr) { if (!buffer.empty()) + { storage_ = allocate_slice>(0, std::move(buffer)); + portion_ = to_byte_span(to_span(static_cast *>(storage_.get())->buffer)); + } } byte_slice::byte_slice(std::initializer_list> sources) diff --git a/tests/unit_tests/epee_utils.cpp b/tests/unit_tests/epee_utils.cpp index a2cec965e..0f91671a7 100644 --- a/tests/unit_tests/epee_utils.cpp +++ b/tests/unit_tests/epee_utils.cpp @@ -387,6 +387,29 @@ TEST(ByteSlice, Construction) EXPECT_FALSE(std::is_copy_assignable()); } +TEST(ByteSlice, DataReturnedMatches) +{ + for (int i = 64; i > 0; i--) + { + std::string sso_string(i, 'a'); + std::string original = sso_string; + epee::byte_slice slice{std::move(sso_string)}; + + EXPECT_EQ(slice.size(), original.size()); + EXPECT_EQ(memcmp(slice.data(), original.data(), original.size()), 0); + } + + for (int i = 64; i > 0; i--) + { + std::vector sso_vector(i, 'a'); + std::vector original = sso_vector; + epee::byte_slice slice{std::move(sso_vector)}; + + EXPECT_EQ(slice.size(), original.size()); + EXPECT_EQ(memcmp(slice.data(), original.data(), original.size()), 0); + } +} + TEST(ByteSlice, NoExcept) { EXPECT_TRUE(std::is_nothrow_default_constructible()); From 81f79d900b8fdaa65f7ee27b2e999bd5989eb5f2 Mon Sep 17 00:00:00 2001 From: rbrunner7 Date: Mon, 4 May 2020 11:31:46 +0200 Subject: [PATCH 12/17] [release-v0.16] MMS: New 'config_checksum' subcommand --- src/simplewallet/simplewallet.cpp | 44 ++++++++++++- src/simplewallet/simplewallet.h | 2 + src/wallet/message_store.cpp | 104 +++++++++++++++++++++++------- src/wallet/message_store.h | 3 +- 4 files changed, 125 insertions(+), 28 deletions(-) diff --git a/src/simplewallet/simplewallet.cpp b/src/simplewallet/simplewallet.cpp index 69d4652bb..f63b2e83a 100644 --- a/src/simplewallet/simplewallet.cpp +++ b/src/simplewallet/simplewallet.cpp @@ -250,6 +250,7 @@ namespace const char* USAGE_MMS_SET("mms set []"); const char* USAGE_MMS_SEND_SIGNER_CONFIG("mms send_signer_config"); const char* USAGE_MMS_START_AUTO_CONFIG("mms start_auto_config [