From f8d05f3cd962fdfe2ad6dbf861188078dbda8df9 Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Sun, 27 Mar 2016 16:25:59 +0100 Subject: [PATCH 1/3] common: new json_util.h With code to help factor out reading typed fields from JSON --- src/common/json_util.h | 53 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 53 insertions(+) create mode 100644 src/common/json_util.h diff --git a/src/common/json_util.h b/src/common/json_util.h new file mode 100644 index 000000000..6f8b4c18f --- /dev/null +++ b/src/common/json_util.h @@ -0,0 +1,53 @@ +// Copyright (c) 2016, 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 + +#define GET_FIELD_FROM_JSON_RETURN_ON_ERROR(json, name, type, jtype, mandatory) \ + type field_##name; \ + bool field_##name##_found = false; \ + (void)field_##name##_found; \ + do if (json.HasMember(#name)) \ + { \ + if (json[#name].Is##jtype()) \ + { \ + field_##name = json[#name].Get##jtype(); \ + field_##name##_found = true; \ + } \ + else \ + { \ + LOG_ERROR("Field " << #name << " found in JSON, but not " << #jtype); \ + return false; \ + } \ + } \ + else if (mandatory) \ + { \ + LOG_ERROR("Field " << #name << " not found in JSON"); \ + return false; \ + } while(0) + From 3e557254c7a0828b0f2ca54aa7181c2dae1d7e4c Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Sun, 27 Mar 2016 16:26:37 +0100 Subject: [PATCH 2/3] wallet: make the JSON reading type safe --- src/simplewallet/simplewallet.cpp | 72 +++++++++++++------------------ 1 file changed, 31 insertions(+), 41 deletions(-) diff --git a/src/simplewallet/simplewallet.cpp b/src/simplewallet/simplewallet.cpp index 04170df62..fbaea1650 100644 --- a/src/simplewallet/simplewallet.cpp +++ b/src/simplewallet/simplewallet.cpp @@ -58,6 +58,7 @@ #include "crypto/crypto.h" // for crypto::secret_key definition #include "mnemonics/electrum-words.h" #include "rapidjson/document.h" +#include "common/json_util.h" #include #if defined(WIN32) @@ -833,40 +834,27 @@ bool simple_wallet::generate_from_json(const boost::program_options::variables_m return false; } - if (!json.HasMember("version")) { - fail_msg_writer() << tr("Version not found in JSON"); - return false; - } - unsigned int version = json["version"].GetUint(); + GET_FIELD_FROM_JSON_RETURN_ON_ERROR(json, version, unsigned, Uint, true); const int current_version = 1; - if (version > current_version) { - fail_msg_writer() << boost::format(tr("Version %u too new, we can only grok up to %u")) % version % current_version; + if (field_version > current_version) { + fail_msg_writer() << boost::format(tr("Version %u too new, we can only grok up to %u")) % field_version % current_version; return false; } - if (!json.HasMember("filename")) { - fail_msg_writer() << tr("Filename not found in JSON"); - return false; - } - std::string filename = json["filename"].GetString(); - bool recover = false; - uint64_t scan_from_height = 0; - if (json.HasMember("scan_from_height")) { - scan_from_height = json["scan_from_height"].GetUint64(); - recover = true; - } + GET_FIELD_FROM_JSON_RETURN_ON_ERROR(json, filename, std::string, String, true); - password = ""; - if (json.HasMember("password")) { - password = json["password"].GetString(); - } + GET_FIELD_FROM_JSON_RETURN_ON_ERROR(json, scan_from_height, uint64_t, Uint64, false); + bool recover = field_scan_from_height_found; - std::string viewkey_string(""); + GET_FIELD_FROM_JSON_RETURN_ON_ERROR(json, password, std::string, String, false); + password = field_password; + + GET_FIELD_FROM_JSON_RETURN_ON_ERROR(json, viewkey, std::string, String, false); crypto::secret_key viewkey; - if (json.HasMember("viewkey")) { - viewkey_string = json["viewkey"].GetString(); + if (field_viewkey_found) + { cryptonote::blobdata viewkey_data; - if(!epee::string_tools::parse_hexstr_to_binbuff(viewkey_string, viewkey_data)) + if(!epee::string_tools::parse_hexstr_to_binbuff(field_viewkey, viewkey_data)) { fail_msg_writer() << tr("failed to parse view key secret key"); return false; @@ -874,12 +862,12 @@ bool simple_wallet::generate_from_json(const boost::program_options::variables_m viewkey = *reinterpret_cast(viewkey_data.data()); } - std::string spendkey_string(""); + GET_FIELD_FROM_JSON_RETURN_ON_ERROR(json, spendkey, std::string, String, false); crypto::secret_key spendkey; - if (json.HasMember("spendkey")) { - spendkey_string = json["spendkey"].GetString(); + if (field_spendkey_found) + { cryptonote::blobdata spendkey_data; - if(!epee::string_tools::parse_hexstr_to_binbuff(spendkey_string, spendkey_data)) + if(!epee::string_tools::parse_hexstr_to_binbuff(field_spendkey, spendkey_data)) { fail_msg_writer() << tr("failed to parse spend key secret key"); return false; @@ -887,30 +875,32 @@ bool simple_wallet::generate_from_json(const boost::program_options::variables_m spendkey = *reinterpret_cast(spendkey_data.data()); } - std::string seed(""); + GET_FIELD_FROM_JSON_RETURN_ON_ERROR(json, seed, std::string, String, false); std::string old_language; - if (json.HasMember("seed")) { - seed = json["seed"].GetString(); - if (!crypto::ElectrumWords::words_to_bytes(seed, m_recovery_key, old_language)) + if (field_seed_found) + { + if (!crypto::ElectrumWords::words_to_bytes(field_seed, m_recovery_key, old_language)) { fail_msg_writer() << tr("Electrum-style word list failed verification"); return false; } - m_electrum_seed = seed; + m_electrum_seed = field_seed; m_restore_deterministic_wallet = true; } // compatibility checks - if (seed.empty() && viewkey_string.empty()) { + if (!field_seed_found && !field_viewkey_found) + { fail_msg_writer() << tr("At least one of Electrum-style word list and private view key must be specified"); return false; } - if (!seed.empty() && (!viewkey_string.empty() || !spendkey_string.empty())) { + if (field_seed_found && (field_viewkey_found || field_spendkey_found)) + { fail_msg_writer() << tr("Both Electrum-style word list and private key(s) specified"); return false; } - m_wallet_file = filename; + m_wallet_file = field_filename; bool was_deprecated_wallet = m_restore_deterministic_wallet && ((old_language == crypto::ElectrumWords::old_language_name) || crypto::ElectrumWords::get_is_old_style_seed(m_electrum_seed)); @@ -925,7 +915,7 @@ bool simple_wallet::generate_from_json(const boost::program_options::variables_m try { - if (!seed.empty()) + if (!field_seed.empty()) { m_wallet->generate(m_wallet_file, password, m_recovery_key, recover, false); } @@ -941,7 +931,7 @@ bool simple_wallet::generate_from_json(const boost::program_options::variables_m return false; } - if (spendkey_string.empty()) + if (field_spendkey.empty()) { m_wallet->generate(m_wallet_file, password, address, viewkey); } @@ -957,7 +947,7 @@ bool simple_wallet::generate_from_json(const boost::program_options::variables_m return false; } - m_wallet->set_refresh_from_block_height(scan_from_height); + m_wallet->set_refresh_from_block_height(field_scan_from_height); wallet_file = m_wallet_file; From b4eada907ca2551d8f0372b40c5af0106b973d50 Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Sun, 27 Mar 2016 22:02:23 +0100 Subject: [PATCH 3/3] wallet: make load_keys check types when loading JSON --- src/wallet/wallet2.cpp | 57 ++++++++++++++++++++++++++---------------- src/wallet/wallet2.h | 2 +- 2 files changed, 37 insertions(+), 22 deletions(-) diff --git a/src/wallet/wallet2.cpp b/src/wallet/wallet2.cpp index 6fd77eead..bfd99d9e3 100644 --- a/src/wallet/wallet2.cpp +++ b/src/wallet/wallet2.cpp @@ -52,6 +52,7 @@ using namespace epee; #include "rapidjson/document.h" #include "rapidjson/writer.h" #include "rapidjson/stringbuffer.h" +#include "common/json_util.h" extern "C" { @@ -1004,7 +1005,7 @@ namespace * \param keys_file_name Name of wallet file * \param password Password of wallet file */ -void wallet2::load_keys(const std::string& keys_file_name, const std::string& password) +bool wallet2::load_keys(const std::string& keys_file_name, const std::string& password) { wallet2::keys_file_data keys_file_data; std::string buf; @@ -1033,34 +1034,44 @@ void wallet2::load_keys(const std::string& keys_file_name, const std::string& pa } else { - account_data = std::string(json["key_data"].GetString(), json["key_data"].GetString() + - json["key_data"].GetStringLength()); - if (json.HasMember("seed_language")) + if (!json.HasMember("key_data")) { - set_seed_language(std::string(json["seed_language"].GetString(), json["seed_language"].GetString() + - json["seed_language"].GetStringLength())); + LOG_ERROR("Field key_data not found in JSON"); + return false; } - if (json.HasMember("watch_only")) + if (!json["key_data"].IsString()) { - m_watch_only = json["watch_only"].GetInt() != 0; + LOG_ERROR("Field key_data found in JSON, but not String"); + return false; } - else + const char *field_key_data = json["key_data"].GetString(); + account_data = std::string(field_key_data, field_key_data + json["key_data"].GetStringLength()); + + GET_FIELD_FROM_JSON_RETURN_ON_ERROR(json, seed_language, std::string, String, false); + if (field_seed_language_found) { - m_watch_only = false; + set_seed_language(field_seed_language); } - m_always_confirm_transfers = json.HasMember("always_confirm_transfers") && (json["always_confirm_transfers"].GetInt() != 0); - m_store_tx_info = (json.HasMember("store_tx_keys") && (json["store_tx_keys"].GetInt() != 0)) - || (json.HasMember("store_tx_info") && (json["store_tx_info"].GetInt() != 0)); - m_default_mixin = json.HasMember("default_mixin") ? json["default_mixin"].GetUint() : 0; - m_auto_refresh = !json.HasMember("auto_refresh") || (json["auto_refresh"].GetInt() != 0); + GET_FIELD_FROM_JSON_RETURN_ON_ERROR(json, watch_only, int, Int, false); + m_watch_only = field_watch_only_found && field_watch_only; + GET_FIELD_FROM_JSON_RETURN_ON_ERROR(json, always_confirm_transfers, int, Int, false); + m_always_confirm_transfers = field_always_confirm_transfers_found && field_always_confirm_transfers; + GET_FIELD_FROM_JSON_RETURN_ON_ERROR(json, store_tx_keys, int, Int, false); + GET_FIELD_FROM_JSON_RETURN_ON_ERROR(json, store_tx_info, int, Int, false); + m_store_tx_info = (field_store_tx_keys_found && (field_store_tx_keys != 0)) + || (field_store_tx_info_found && (field_store_tx_info != 0)); + GET_FIELD_FROM_JSON_RETURN_ON_ERROR(json, default_mixin, unsigned int, Uint, false); + m_default_mixin = field_default_mixin_found ? field_default_mixin : 0; + GET_FIELD_FROM_JSON_RETURN_ON_ERROR(json, auto_refresh, int, Int, false); + m_auto_refresh = !field_auto_refresh_found || (field_auto_refresh != 0); + GET_FIELD_FROM_JSON_RETURN_ON_ERROR(json, refresh_type, int, Int, false); m_refresh_type = RefreshType::RefreshDefault; - if (json.HasMember("refresh_type")) + if (field_refresh_type_found) { - int type = json["refresh_type"].GetInt(); - if (type == RefreshFull || type == RefreshOptimizeCoinbase || type == RefreshNoCoinbase) - m_refresh_type = (RefreshType)type; + if (field_refresh_type == RefreshFull || field_refresh_type == RefreshOptimizeCoinbase || field_refresh_type == RefreshNoCoinbase) + m_refresh_type = (RefreshType)field_refresh_type; else - LOG_PRINT_L0("Unknown refresh-type value (" << type << "), using default"); + LOG_PRINT_L0("Unknown refresh-type value (" << field_refresh_type << "), using default"); } } @@ -1070,6 +1081,7 @@ void wallet2::load_keys(const std::string& keys_file_name, const std::string& pa if(!m_watch_only) r = r && verify_keys(keys.m_spend_secret_key, keys.m_account_address.m_spend_public_key); THROW_WALLET_EXCEPTION_IF(!r, error::invalid_password); + return true; } /*! @@ -1358,7 +1370,10 @@ void wallet2::load(const std::string& wallet_, const std::string& password) bool exists = boost::filesystem::exists(m_keys_file, e); THROW_WALLET_EXCEPTION_IF(e || !exists, error::file_not_found, m_keys_file); - load_keys(m_keys_file, password); + if (!load_keys(m_keys_file, password)) + { + THROW_WALLET_EXCEPTION_IF(true, error::file_read_error, m_keys_file); + } LOG_PRINT_L0("Loaded wallet keys file, with public address: " << m_account.get_public_address_str(m_testnet)); //keys loaded ok! diff --git a/src/wallet/wallet2.h b/src/wallet/wallet2.h index fc700a3de..b6466d3f6 100644 --- a/src/wallet/wallet2.h +++ b/src/wallet/wallet2.h @@ -378,7 +378,7 @@ namespace tools * \param keys_file_name Name of wallet file * \param password Password of wallet file */ - void load_keys(const std::string& keys_file_name, const std::string& password); + bool load_keys(const std::string& keys_file_name, const std::string& password); void process_new_transaction(const cryptonote::transaction& tx, uint64_t height, bool miner_tx); void process_new_blockchain_entry(const cryptonote::block& b, const cryptonote::block_complete_entry& bche, const crypto::hash& bl_id, uint64_t height); void detach_blockchain(uint64_t height);