Fixes #1497 ZCA-009 by restricting data exporting to user defined folder.
Previously the RPC interface allowed z_exportwallet, backupwallet and dumpwallet to write data to an arbitrary filename. ZCA-009 demonstrates how this is vulnerable. The resolution is to only allow data to written when the -exportdir has been configured. Also filenames are restricted to alphanumeric characters.
This commit is contained in:
parent
57a0725ae1
commit
9064d73bf8
|
@ -289,6 +289,7 @@ std::string HelpMessage(HelpMessageMode mode)
|
||||||
#endif
|
#endif
|
||||||
}
|
}
|
||||||
strUsage += HelpMessageOpt("-datadir=<dir>", _("Specify data directory"));
|
strUsage += HelpMessageOpt("-datadir=<dir>", _("Specify data directory"));
|
||||||
|
strUsage += HelpMessageOpt("-exportdir=<dir>", _("Specify directory to be used when exporting data"));
|
||||||
strUsage += HelpMessageOpt("-dbcache=<n>", strprintf(_("Set database cache size in megabytes (%d to %d, default: %d)"), nMinDbCache, nMaxDbCache, nDefaultDbCache));
|
strUsage += HelpMessageOpt("-dbcache=<n>", strprintf(_("Set database cache size in megabytes (%d to %d, default: %d)"), nMinDbCache, nMaxDbCache, nDefaultDbCache));
|
||||||
strUsage += HelpMessageOpt("-loadblock=<file>", _("Imports blocks from external blk000??.dat file") + " " + _("on startup"));
|
strUsage += HelpMessageOpt("-loadblock=<file>", _("Imports blocks from external blk000??.dat file") + " " + _("on startup"));
|
||||||
strUsage += HelpMessageOpt("-maxorphantx=<n>", strprintf(_("Keep at most <n> unconnectable transactions in memory (default: %u)"), DEFAULT_MAX_ORPHAN_TRANSACTIONS));
|
strUsage += HelpMessageOpt("-maxorphantx=<n>", strprintf(_("Keep at most <n> unconnectable transactions in memory (default: %u)"), DEFAULT_MAX_ORPHAN_TRANSACTIONS));
|
||||||
|
|
|
@ -358,16 +358,26 @@ BOOST_AUTO_TEST_CASE(rpc_wallet_z_exportwallet)
|
||||||
pwalletMain->GetPaymentAddresses(addrs);
|
pwalletMain->GetPaymentAddresses(addrs);
|
||||||
BOOST_CHECK(addrs.size()==1);
|
BOOST_CHECK(addrs.size()==1);
|
||||||
|
|
||||||
|
// Set up paths
|
||||||
|
boost::filesystem::path tmppath = boost::filesystem::temp_directory_path();
|
||||||
|
boost::filesystem::path tmpfilename = boost::filesystem::unique_path("%%%%%%%%");
|
||||||
|
boost::filesystem::path exportfilepath = tmppath / tmpfilename;
|
||||||
|
|
||||||
|
// export will fail since exportdir is not set
|
||||||
|
BOOST_CHECK_THROW(CallRPC(string("z_exportwallet ") + tmpfilename.string()), runtime_error);
|
||||||
|
|
||||||
|
// set exportdir
|
||||||
|
mapArgs["-exportdir"] = tmppath.native();
|
||||||
|
|
||||||
|
// run some tests
|
||||||
BOOST_CHECK_THROW(CallRPC("z_exportwallet"), runtime_error);
|
BOOST_CHECK_THROW(CallRPC("z_exportwallet"), runtime_error);
|
||||||
|
|
||||||
BOOST_CHECK_THROW(CallRPC("z_exportwallet toomany args"), runtime_error);
|
BOOST_CHECK_THROW(CallRPC("z_exportwallet toomany args"), runtime_error);
|
||||||
|
|
||||||
|
BOOST_CHECK_THROW(CallRPC(string("z_exportwallet invalid!*/_chars.txt")), runtime_error);
|
||||||
boost::filesystem::path temp = boost::filesystem::temp_directory_path() /
|
|
||||||
boost::filesystem::unique_path();
|
BOOST_CHECK_NO_THROW(CallRPC(string("z_exportwallet ") + tmpfilename.string()));
|
||||||
const std::string path = temp.native();
|
|
||||||
|
|
||||||
BOOST_CHECK_NO_THROW(CallRPC(string("z_exportwallet ") + path));
|
|
||||||
|
|
||||||
auto addr = paymentAddress.Get();
|
auto addr = paymentAddress.Get();
|
||||||
libzcash::SpendingKey key;
|
libzcash::SpendingKey key;
|
||||||
|
@ -382,7 +392,7 @@ BOOST_AUTO_TEST_CASE(rpc_wallet_z_exportwallet)
|
||||||
EnsureWalletIsUnlocked();
|
EnsureWalletIsUnlocked();
|
||||||
|
|
||||||
ifstream file;
|
ifstream file;
|
||||||
file.open(path.c_str(), std::ios::in | std::ios::ate);
|
file.open(exportfilepath.string().c_str(), std::ios::in | std::ios::ate);
|
||||||
BOOST_CHECK(file.is_open());
|
BOOST_CHECK(file.is_open());
|
||||||
bool fVerified = false;
|
bool fVerified = false;
|
||||||
int64_t nFilesize = std::max((int64_t)1, (int64_t)file.tellg());
|
int64_t nFilesize = std::max((int64_t)1, (int64_t)file.tellg());
|
||||||
|
|
20
src/util.cpp
20
src/util.cpp
|
@ -484,6 +484,26 @@ const boost::filesystem::path &ZC_GetParamsDir()
|
||||||
return path;
|
return path;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Return the user specified export directory. Create directory if it doesn't exist.
|
||||||
|
// If user did not set option, return an empty path.
|
||||||
|
// If there is a filesystem problem, throw an exception.
|
||||||
|
const boost::filesystem::path GetExportDir()
|
||||||
|
{
|
||||||
|
namespace fs = boost::filesystem;
|
||||||
|
fs::path path;
|
||||||
|
if (mapArgs.count("-exportdir")) {
|
||||||
|
path = fs::system_complete(mapArgs["-exportdir"]);
|
||||||
|
if (fs::exists(path) && !fs::is_directory(path)) {
|
||||||
|
throw std::runtime_error(strprintf("The -exportdir '%s' already exists and is not a directory", path.string()));
|
||||||
|
}
|
||||||
|
if (!fs::exists(path) && !fs::create_directories(path)) {
|
||||||
|
throw std::runtime_error(strprintf("Failed to create directory at -exportdir '%s'", path.string()));
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return path;
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
const boost::filesystem::path &GetDataDir(bool fNetSpecific)
|
const boost::filesystem::path &GetDataDir(bool fNetSpecific)
|
||||||
{
|
{
|
||||||
namespace fs = boost::filesystem;
|
namespace fs = boost::filesystem;
|
||||||
|
|
|
@ -134,6 +134,7 @@ boost::filesystem::path GetSpecialFolderPath(int nFolder, bool fCreate = true);
|
||||||
boost::filesystem::path GetTempPath();
|
boost::filesystem::path GetTempPath();
|
||||||
void ShrinkDebugFile();
|
void ShrinkDebugFile();
|
||||||
void runCommand(std::string strCommand);
|
void runCommand(std::string strCommand);
|
||||||
|
const boost::filesystem::path GetExportDir();
|
||||||
|
|
||||||
inline bool IsSwitchChar(char c)
|
inline bool IsSwitchChar(char c)
|
||||||
{
|
{
|
||||||
|
|
|
@ -30,6 +30,22 @@ string SanitizeString(const string& str)
|
||||||
return strResult;
|
return strResult;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
string SanitizeFilename(const string& str)
|
||||||
|
{
|
||||||
|
/**
|
||||||
|
* safeChars chosen to restrict filename, keeping it simple to avoid cross-platform issues.
|
||||||
|
* http://stackoverflow.com/a/2306003
|
||||||
|
*/
|
||||||
|
static string safeChars("abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ01234567890");
|
||||||
|
string strResult;
|
||||||
|
for (std::string::size_type i = 0; i < str.size(); i++)
|
||||||
|
{
|
||||||
|
if (safeChars.find(str[i]) != std::string::npos)
|
||||||
|
strResult.push_back(str[i]);
|
||||||
|
}
|
||||||
|
return strResult;
|
||||||
|
}
|
||||||
|
|
||||||
const signed char p_util_hexdigit[256] =
|
const signed char p_util_hexdigit[256] =
|
||||||
{ -1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,
|
{ -1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,
|
||||||
-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,
|
-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,
|
||||||
|
|
|
@ -22,6 +22,7 @@
|
||||||
/** This is needed because the foreach macro can't get over the comma in pair<t1, t2> */
|
/** This is needed because the foreach macro can't get over the comma in pair<t1, t2> */
|
||||||
#define PAIRTYPE(t1, t2) std::pair<t1, t2>
|
#define PAIRTYPE(t1, t2) std::pair<t1, t2>
|
||||||
|
|
||||||
|
std::string SanitizeFilename(const std::string& str);
|
||||||
std::string SanitizeString(const std::string& str);
|
std::string SanitizeString(const std::string& str);
|
||||||
std::vector<unsigned char> ParseHex(const char* psz);
|
std::vector<unsigned char> ParseHex(const char* psz);
|
||||||
std::vector<unsigned char> ParseHex(const std::string& str);
|
std::vector<unsigned char> ParseHex(const std::string& str);
|
||||||
|
|
|
@ -430,7 +430,9 @@ Value z_exportwallet(const Array& params, bool fHelp)
|
||||||
"z_exportwallet \"filename\"\n"
|
"z_exportwallet \"filename\"\n"
|
||||||
"\nExports all wallet keys, for taddr and zaddr, in a human-readable format.\n"
|
"\nExports all wallet keys, for taddr and zaddr, in a human-readable format.\n"
|
||||||
"\nArguments:\n"
|
"\nArguments:\n"
|
||||||
"1. \"filename\" (string, required) The filename\n"
|
"1. \"filename\" (string, required) The filename, saved in folder set by zcashd -exportdir option\n"
|
||||||
|
"\nResult:\n"
|
||||||
|
"\"path\" (string) The full path of the destination file\n"
|
||||||
"\nExamples:\n"
|
"\nExamples:\n"
|
||||||
+ HelpExampleCli("z_exportwallet", "\"test\"")
|
+ HelpExampleCli("z_exportwallet", "\"test\"")
|
||||||
+ HelpExampleRpc("z_exportwallet", "\"test\"")
|
+ HelpExampleRpc("z_exportwallet", "\"test\"")
|
||||||
|
@ -449,7 +451,9 @@ Value dumpwallet(const Array& params, bool fHelp)
|
||||||
"dumpwallet \"filename\"\n"
|
"dumpwallet \"filename\"\n"
|
||||||
"\nDumps taddr wallet keys in a human-readable format.\n"
|
"\nDumps taddr wallet keys in a human-readable format.\n"
|
||||||
"\nArguments:\n"
|
"\nArguments:\n"
|
||||||
"1. \"filename\" (string, required) The filename\n"
|
"1. \"filename\" (string, required) The filename, saved in folder set by zcashd -exportdir option\n"
|
||||||
|
"\nResult:\n"
|
||||||
|
"\"path\" (string) The full path of the destination file\n"
|
||||||
"\nExamples:\n"
|
"\nExamples:\n"
|
||||||
+ HelpExampleCli("dumpwallet", "\"test\"")
|
+ HelpExampleCli("dumpwallet", "\"test\"")
|
||||||
+ HelpExampleRpc("dumpwallet", "\"test\"")
|
+ HelpExampleRpc("dumpwallet", "\"test\"")
|
||||||
|
@ -464,8 +468,24 @@ Value dumpwallet_impl(const Array& params, bool fHelp, bool fDumpZKeys)
|
||||||
|
|
||||||
EnsureWalletIsUnlocked();
|
EnsureWalletIsUnlocked();
|
||||||
|
|
||||||
|
boost::filesystem::path exportdir;
|
||||||
|
try {
|
||||||
|
exportdir = GetExportDir();
|
||||||
|
} catch (const std::runtime_error& e) {
|
||||||
|
throw JSONRPCError(RPC_INTERNAL_ERROR, e.what());
|
||||||
|
}
|
||||||
|
if (exportdir.empty()) {
|
||||||
|
throw JSONRPCError(RPC_WALLET_ERROR, "Cannot export wallet until the zcashd -exportdir option has been set");
|
||||||
|
}
|
||||||
|
std::string unclean = params[0].get_str();
|
||||||
|
std::string clean = SanitizeFilename(unclean);
|
||||||
|
if (clean.compare(unclean) != 0) {
|
||||||
|
throw JSONRPCError(RPC_WALLET_ERROR, strprintf("Filename is invalid as only alphanumeric characters are allowed. Try '%s' instead.", clean));
|
||||||
|
}
|
||||||
|
boost::filesystem::path exportfilepath = exportdir / clean;
|
||||||
|
|
||||||
ofstream file;
|
ofstream file;
|
||||||
file.open(params[0].get_str().c_str());
|
file.open(exportfilepath.string().c_str());
|
||||||
if (!file.is_open())
|
if (!file.is_open())
|
||||||
throw JSONRPCError(RPC_INVALID_PARAMETER, "Cannot open wallet dump file");
|
throw JSONRPCError(RPC_INVALID_PARAMETER, "Cannot open wallet dump file");
|
||||||
|
|
||||||
|
@ -523,7 +543,8 @@ Value dumpwallet_impl(const Array& params, bool fHelp, bool fDumpZKeys)
|
||||||
|
|
||||||
file << "# End of dump\n";
|
file << "# End of dump\n";
|
||||||
file.close();
|
file.close();
|
||||||
return Value::null;
|
|
||||||
|
return exportfilepath.string();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
|
|
|
@ -1766,21 +1766,38 @@ Value backupwallet(const Array& params, bool fHelp)
|
||||||
if (fHelp || params.size() != 1)
|
if (fHelp || params.size() != 1)
|
||||||
throw runtime_error(
|
throw runtime_error(
|
||||||
"backupwallet \"destination\"\n"
|
"backupwallet \"destination\"\n"
|
||||||
"\nSafely copies wallet.dat to destination, which can be a directory or a path with filename.\n"
|
"\nSafely copies wallet.dat to destination filename\n"
|
||||||
"\nArguments:\n"
|
"\nArguments:\n"
|
||||||
"1. \"destination\" (string) The destination directory or file\n"
|
"1. \"destination\" (string, required) The destination filename, saved in the directory set by -exportdir option.\n"
|
||||||
|
"\nResult:\n"
|
||||||
|
"\"path\" (string) The full path of the destination file\n"
|
||||||
"\nExamples:\n"
|
"\nExamples:\n"
|
||||||
+ HelpExampleCli("backupwallet", "\"backup.dat\"")
|
+ HelpExampleCli("backupwallet", "\"backupdata\"")
|
||||||
+ HelpExampleRpc("backupwallet", "\"backup.dat\"")
|
+ HelpExampleRpc("backupwallet", "\"backupdata\"")
|
||||||
);
|
);
|
||||||
|
|
||||||
LOCK2(cs_main, pwalletMain->cs_wallet);
|
LOCK2(cs_main, pwalletMain->cs_wallet);
|
||||||
|
|
||||||
string strDest = params[0].get_str();
|
boost::filesystem::path exportdir;
|
||||||
if (!BackupWallet(*pwalletMain, strDest))
|
try {
|
||||||
|
exportdir = GetExportDir();
|
||||||
|
} catch (const std::runtime_error& e) {
|
||||||
|
throw JSONRPCError(RPC_INTERNAL_ERROR, e.what());
|
||||||
|
}
|
||||||
|
if (exportdir.empty()) {
|
||||||
|
throw JSONRPCError(RPC_WALLET_ERROR, "Cannot backup wallet until the -exportdir option has been set");
|
||||||
|
}
|
||||||
|
std::string unclean = params[0].get_str();
|
||||||
|
std::string clean = SanitizeFilename(unclean);
|
||||||
|
if (clean.compare(unclean) != 0) {
|
||||||
|
throw JSONRPCError(RPC_WALLET_ERROR, strprintf("Filename is invalid as only alphanumeric characters are allowed. Try '%s' instead.", clean));
|
||||||
|
}
|
||||||
|
boost::filesystem::path exportfilepath = exportdir / clean;
|
||||||
|
|
||||||
|
if (!BackupWallet(*pwalletMain, exportfilepath.string()))
|
||||||
throw JSONRPCError(RPC_WALLET_ERROR, "Error: Wallet backup failed!");
|
throw JSONRPCError(RPC_WALLET_ERROR, "Error: Wallet backup failed!");
|
||||||
|
|
||||||
return Value::null;
|
return exportfilepath.string();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
|
|
Loading…
Reference in New Issue