[Pkg-bitcoin-commits] [bitcoin] 57/126: rpc: Prevent `dumpwallet` from overwriting files
Jonas Smedegaard
dr at jones.dk
Mon Nov 13 20:01:58 UTC 2017
This is an automated email from the git hooks/post-receive script.
js pushed a commit to annotated tag debian/0.15.1_dfsg-1
in repository bitcoin.
commit a43be5bcdb27a68abe9bb5fec57185a1b6652479
Author: Wladimir J. van der Laan <laanwj at gmail.com>
Date: Tue Mar 7 09:50:41 2017 +0100
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: 0cd9273fd959c6742574259d026039f7da0309a2
---
doc/release-notes.md | 3 +++
src/wallet/rpcdump.cpp | 14 ++++++++++++--
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 ef8de31..0235e1c 100644
--- a/doc/release-notes.md
+++ b/doc/release-notes.md
@@ -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
=======
diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp
index 67c6d9e..e56485e 100644
--- a/src/wallet/rpcdump.cpp
+++ b/src/wallet/rpcdump.cpp
@@ -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");
diff --git a/test/functional/wallet-dump.py b/test/functional/wallet-dump.py
index 569cc46..016bd95 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):
@@ -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 ()
--
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/pkg-bitcoin/bitcoin.git
More information about the Pkg-bitcoin-commits
mailing list