diff options
| -rw-r--r-- | doc/release-notes.md | 3 | ||||
| -rw-r--r-- | src/wallet/rpcdump.cpp | 14 | ||||
| -rwxr-xr-x | test/functional/wallet-dump.py | 5 |
3 files changed, 19 insertions, 3 deletions
diff --git a/doc/release-notes.md b/doc/release-notes.md index 04fb0f333b..4ecca7897c 100644 --- a/doc/release-notes.md +++ b/doc/release-notes.md @@ -83,6 +83,9 @@ Low-level RPC changes * `getwalletinfo` * `getmininginfo` +- `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 9539cc9f42..1123fd6dbb 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -600,7 +600,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 with path (either absolute or relative to bitcoind)\n" "\nResult:\n" @@ -616,9 +616,19 @@ UniValue dumpwallet(const JSONRPCRequest& request) EnsureWalletIsUnlocked(pwallet); - std::ofstream file; 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(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 e4757c8c03..12db95e5d9 100755 --- a/test/functional/wallet-dump.py +++ b/test/functional/wallet-dump.py @@ -7,7 +7,7 @@ import os from test_framework.test_framework import BitcoinTestFramework -from test_framework.util import assert_equal +from test_framework.util import (assert_equal, assert_raises_jsonrpc) def read_dump(file_name, addrs, hd_master_addr_old): @@ -105,5 +105,8 @@ class WalletDumpTest(BitcoinTestFramework): assert_equal(found_addr_chg, 90*2 + 50) # old reserve keys are marked as change now assert_equal(found_addr_rsv, 90*2) + # Overwriting should fail + assert_raises_jsonrpc(-8, "already exists", self.nodes[0].dumpwallet, tmpdir + "/node0/wallet.unencrypted.dump") + if __name__ == '__main__': WalletDumpTest().main () |