Merge pull request #15431 from SimonWalton-HiFi/readkeys-double-free

Don't double-free an openssl structure in readKeys()
This commit is contained in:
Simon Walton 2019-04-23 12:06:19 -07:00 committed by GitHub
commit db411cc456
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23

View file

@ -41,10 +41,11 @@
#include "ui/SecurityImageProvider.h"
#include "scripting/HMDScriptingInterface.h"
static const char* KEY_FILE = "hifikey";
static const char* INSTRUCTIONS_FILE = "backup_instructions.html";
static const char* IMAGE_HEADER = "-----BEGIN SECURITY IMAGE-----\n";
static const char* IMAGE_FOOTER = "-----END SECURITY IMAGE-----\n";
namespace {
const char* KEY_FILE = "hifikey";
const char* INSTRUCTIONS_FILE = "backup_instructions.html";
const char* IMAGE_HEADER = "-----BEGIN SECURITY IMAGE-----\n";
const char* IMAGE_FOOTER = "-----END SECURITY IMAGE-----\n";
void initialize() {
static bool initialized = false;
@ -60,23 +61,6 @@ QString keyFilePath() {
auto accountManager = DependencyManager::get<AccountManager>();
return PathUtils::getAppDataFilePath(QString("%1.%2").arg(accountManager->getAccountInfo().getUsername(), KEY_FILE));
}
bool Wallet::copyKeyFileFrom(const QString& pathname) {
QString existing = getKeyFilePath();
qCDebug(commerce) << "Old keyfile" << existing;
if (!existing.isEmpty()) {
QString backup = QString(existing).insert(existing.indexOf(KEY_FILE) - 1,
QDateTime::currentDateTime().toString(Qt::ISODate).replace(":", ""));
qCDebug(commerce) << "Renaming old keyfile to" << backup;
if (!QFile::rename(existing, backup)) {
qCCritical(commerce) << "Unable to backup" << existing << "to" << backup;
return false;
}
}
QString destination = keyFilePath();
bool result = QFile::copy(pathname, destination);
qCDebug(commerce) << "copy" << pathname << "to" << destination << "=>" << result;
return result;
}
// use the cached _passphrase if it exists, otherwise we need to prompt
int passwordCallback(char* password, int maxPasswordSize, int rwFlag, void* u) {
@ -112,8 +96,6 @@ EC_KEY* readKeys(QString filename) {
if ((key = PEM_read_bio_ECPrivateKey(bufio, &key, passwordCallback, NULL))) {
qCDebug(commerce) << "read private key";
BIO_free(bufio);
file.close();
} else {
qCDebug(commerce) << "failed to read private key";
}
@ -128,46 +110,6 @@ EC_KEY* readKeys(QString filename) {
return key;
}
bool Wallet::writeBackupInstructions() {
QString inputFilename(PathUtils::resourcesPath() + "html/commerce/backup_instructions.html");
QString outputFilename = PathUtils::getAppDataFilePath(INSTRUCTIONS_FILE);
QFile inputFile(inputFilename);
QFile outputFile(outputFilename);
bool retval = false;
if (getKeyFilePath().isEmpty()) {
return false;
}
if (QFile::exists(inputFilename) && inputFile.open(QIODevice::ReadOnly)) {
if (outputFile.open(QIODevice::ReadWrite)) {
// Read the data from the original file, then close it
QByteArray fileData = inputFile.readAll();
inputFile.close();
// Translate the data from the original file into a QString
QString text(fileData);
// Replace the necessary string
text.replace(QString("HIFIKEY_PATH_REPLACEME"), keyFilePath());
// Write the new text back to the file
outputFile.write(text.toUtf8());
// Close the output file
outputFile.close();
retval = true;
qCDebug(commerce) << "wrote html file successfully";
} else {
qCDebug(commerce) << "failed to open output html file" << outputFilename;
}
} else {
qCDebug(commerce) << "failed to open input html file" << inputFilename;
}
return retval;
}
bool writeKeys(QString filename, EC_KEY* keys) {
BIO* bio = BIO_new(BIO_s_mem());
bool retval = false;
@ -200,30 +142,6 @@ bool writeKeys(QString filename, EC_KEY* keys) {
return retval;
}
bool Wallet::setWallet(const QByteArray& wallet) {
QFile file(keyFilePath());
if (!file.open(QIODevice::WriteOnly)) {
qCCritical(commerce) << "Unable to open wallet for write in" << keyFilePath();
return false;
}
if (file.write(wallet) != wallet.count()) {
qCCritical(commerce) << "Unable to write wallet in" << keyFilePath();
return false;
}
file.close();
return true;
}
QByteArray Wallet::getWallet() {
QFile file(keyFilePath());
if (!file.open(QIODevice::ReadOnly)) {
qCInfo(commerce) << "No existing wallet in" << keyFilePath();
return QByteArray();
}
QByteArray wallet = file.readAll();
file.close();
return wallet;
}
QPair<QByteArray*, QByteArray*> generateECKeypair() {
EC_KEY* keyPair = EC_KEY_new_by_curve_name(NID_secp256k1);
QPair<QByteArray*, QByteArray*> retval {};
@ -362,6 +280,8 @@ void initializeAESKeys(unsigned char* ivec, unsigned char* ckey, const QByteArra
memcpy(ckey, wallet->getCKey(), 32);
}
} // close unnamed namespace
Wallet::Wallet() {
auto nodeList = DependencyManager::get<NodeList>();
auto ledger = DependencyManager::get<Ledger>();
@ -423,6 +343,88 @@ Wallet::~Wallet() {
}
}
bool Wallet::setWallet(const QByteArray& wallet) {
QFile file(keyFilePath());
if (!file.open(QIODevice::WriteOnly)) {
qCCritical(commerce) << "Unable to open wallet for write in" << keyFilePath();
return false;
}
if (file.write(wallet) != wallet.count()) {
qCCritical(commerce) << "Unable to write wallet in" << keyFilePath();
return false;
}
file.close();
return true;
}
QByteArray Wallet::getWallet() {
QFile file(keyFilePath());
if (!file.open(QIODevice::ReadOnly)) {
qCInfo(commerce) << "No existing wallet in" << keyFilePath();
return QByteArray();
}
QByteArray wallet = file.readAll();
file.close();
return wallet;
}
bool Wallet::copyKeyFileFrom(const QString& pathname) {
QString existing = getKeyFilePath();
qCDebug(commerce) << "Old keyfile" << existing;
if (!existing.isEmpty()) {
QString backup = QString(existing).insert(existing.indexOf(KEY_FILE) - 1,
QDateTime::currentDateTime().toString(Qt::ISODate).replace(":", ""));
qCDebug(commerce) << "Renaming old keyfile to" << backup;
if (!QFile::rename(existing, backup)) {
qCCritical(commerce) << "Unable to backup" << existing << "to" << backup;
return false;
}
}
QString destination = keyFilePath();
bool result = QFile::copy(pathname, destination);
qCDebug(commerce) << "copy" << pathname << "to" << destination << "=>" << result;
return result;
}
bool Wallet::writeBackupInstructions() {
QString inputFilename(PathUtils::resourcesPath() + "html/commerce/backup_instructions.html");
QString outputFilename = PathUtils::getAppDataFilePath(INSTRUCTIONS_FILE);
QFile inputFile(inputFilename);
QFile outputFile(outputFilename);
bool retval = false;
if (getKeyFilePath().isEmpty()) {
return false;
}
if (QFile::exists(inputFilename) && inputFile.open(QIODevice::ReadOnly)) {
if (outputFile.open(QIODevice::ReadWrite)) {
// Read the data from the original file, then close it
QByteArray fileData = inputFile.readAll();
inputFile.close();
// Translate the data from the original file into a QString
QString text(fileData);
// Replace the necessary string
text.replace(QString("HIFIKEY_PATH_REPLACEME"), keyFilePath());
// Write the new text back to the file
outputFile.write(text.toUtf8());
// Close the output file
outputFile.close();
retval = true;
qCDebug(commerce) << "wrote html file successfully";
} else {
qCDebug(commerce) << "failed to open output html file" << outputFilename;
}
} else {
qCDebug(commerce) << "failed to open input html file" << inputFilename;
}
return retval;
}
bool Wallet::setPassphrase(const QString& passphrase) {
if (_passphrase) {
delete _passphrase;