rpc: Prevent dumpwallet from overwriting files

Prevent arbitrary files from being overwritten. There have been reports
that users have overwritten wallet files this way. It may also avoid
other security issues.

Fixes #9934. Adds mention to release notes and adds a test.
This commit is contained in:
Wladimir J. van der Laan 2017-03-07 09:50:41 +01:00
parent 94c9015bca
commit 0cd9273fd9
3 changed files with 19 additions and 3 deletions

View file

@ -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
=======

View file

@ -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");

View file

@ -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 ()