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:
parent
94c9015bca
commit
0cd9273fd9
3 changed files with 19 additions and 3 deletions
|
@ -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
|
||||
=======
|
||||
|
||||
|
|
|
@ -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");
|
||||
|
|
|
@ -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 ()
|
||||
|
|
Loading…
Reference in a new issue