Browse Source

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.

Github-Pull: #9937
Rebased-From: 0cd9273fd9
tags/v0.15.1
Wladimir J. van der Laan 4 years ago
parent
commit
a43be5bcdb
3 changed files with 19 additions and 3 deletions
  1. 3
    0
      doc/release-notes.md
  2. 12
    2
      src/wallet/rpcdump.cpp
  3. 4
    1
      test/functional/wallet-dump.py

+ 3
- 0
doc/release-notes.md View File

@@ -65,6 +65,9 @@ Notable changes
0.15.1 Change log
=================

- `dumpwallet` no longer allows overwriting files. This is a security measure
as well as prevents dangerous user mistakes.

Credits
=======


+ 12
- 2
src/wallet/rpcdump.cpp View File

@@ -595,7 +595,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"
@@ -611,9 +611,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");

+ 4
- 1
test/functional/wallet-dump.py 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):
@@ -108,5 +108,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 ()

Loading…
Cancel
Save