diff --git a/doc/release-notes.md b/doc/release-notes.md index ab7b4288da..61d1bc256e 100644 --- a/doc/release-notes.md +++ b/doc/release-notes.md @@ -109,6 +109,9 @@ See detailed [set of changes](https://github.com/dashpay/dash/compare/v0.14.0.2. - [`4286dde49`](https://github.com/dashpay/dash/commit/4286dde49) Remove support for InstantSend locked gobject collaterals (#3019) - [`788d42dbc`](https://github.com/dashpay/dash/commit/788d42dbc) Bump version to 0.14.0.3 and copy release notes (#3053) +- `dumpwallet` no longer allows overwriting files. This is a security measure + as well as prevents dangerous user mistakes. + Credits ======= diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index 428fdffd38..d2c666936d 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -774,7 +774,7 @@ UniValue dumpwallet(const JSONRPCRequest& request) if (request.fHelp || request.params.size() != 1) throw std::runtime_error( "dumpwallet \"filename\"\n" - "\nDumps all wallet keys in a human-readable format.\n" + "\nDumps all wallet keys in a human-readable format to a server-side file. This does not allow overwriting existing files.\n" "\nArguments:\n" "1. \"filename\" (string, required) The filename\n" "\nExamples:\n" @@ -786,8 +786,20 @@ UniValue dumpwallet(const JSONRPCRequest& request) EnsureWalletIsUnlocked(pwallet); + boost::filesystem::path filepath = request.params[0].get_str(); + filepath = boost::filesystem::absolute(filepath); + + /* Prevent arbitrary files from being overwritten. There have been reports + * that users have overwritten wallet files this way: + * https://github.com/bitcoin/bitcoin/issues/9934 + * It may also avoid other security issues. + */ + if (boost::filesystem::exists(filepath)) { + throw JSONRPCError(RPC_INVALID_PARAMETER, filepath.string() + " already exists. If you are sure this is what you want, move it out of the way first"); + } + std::ofstream file; - file.open(request.params[0].get_str().c_str()); + file.open(filepath.string().c_str()); if (!file.is_open()) throw JSONRPCError(RPC_INVALID_PARAMETER, "Cannot open wallet dump file"); diff --git a/test/functional/wallet-dump.py b/test/functional/wallet-dump.py index 0c6bd2f1de..68452f5cbf 100755 --- a/test/functional/wallet-dump.py +++ b/test/functional/wallet-dump.py @@ -6,7 +6,7 @@ import sys from test_framework.test_framework import BitcoinTestFramework -from test_framework.util import * +from test_framework.util import (assert_equal, assert_raises_jsonrpc) def read_dump(file_name, addrs, hd_master_addr_old): @@ -111,5 +111,8 @@ class WalletDumpTest(BitcoinTestFramework): # assert_equal(found_addr_chg, 180 + 50) # old reserve keys are marked as change now assert_equal(found_addr_rsv, 180) # keypool size + # Overwriting should fail + assert_raises_jsonrpc(-8, "already exists", self.nodes[0].dumpwallet, tmpdir + "/node0/wallet.unencrypted.dump") + if __name__ == '__main__': WalletDumpTest().main ()