gui: Fix proxy setting options dialog crash

This fixes a crash bug when opening the options dialog.

- Check the return value of split() to avoid segmentation faults due to
  out of bounds when the user manages to enter invalid proxy settings.
  This is reported resonably often.

- Move the default proxy/port to a constant instead of hardcoding magic
  values.

- Factor out some common code.

- Revert #11448 because this proves a more robust replacement, it is no
  longer necessary and didn't generally solve the issue.

No attempt is made to do full sanity checking on the proxy, so it can
still be rejected by the core with an InitError message.
This commit is contained in:
Wladimir J. van der Laan 2017-12-01 12:08:31 +01:00
parent 16fff80257
commit f05d349887
3 changed files with 61 additions and 53 deletions

View File

@ -338,7 +338,7 @@ QValidator::State ProxyAddressValidator::validate(QString &input, int &pos) cons
{ {
Q_UNUSED(pos); Q_UNUSED(pos);
// Validate the proxy // Validate the proxy
CService serv(LookupNumeric(input.toStdString().c_str(), 9050)); CService serv(LookupNumeric(input.toStdString().c_str(), DEFAULT_GUI_PROXY_PORT));
proxyType addrProxy = proxyType(serv, true); proxyType addrProxy = proxyType(serv, true);
if (addrProxy.IsValid()) if (addrProxy.IsValid())
return QValidator::Acceptable; return QValidator::Acceptable;

View File

@ -28,6 +28,8 @@
#include <QSettings> #include <QSettings>
#include <QStringList> #include <QStringList>
const char *DEFAULT_GUI_PROXY_HOST = "127.0.0.1";
OptionsModel::OptionsModel(QObject *parent, bool resetSettings) : OptionsModel::OptionsModel(QObject *parent, bool resetSettings) :
QAbstractListModel(parent) QAbstractListModel(parent)
{ {
@ -124,8 +126,8 @@ void OptionsModel::Init(bool resetSettings)
if (!settings.contains("fUseProxy")) if (!settings.contains("fUseProxy"))
settings.setValue("fUseProxy", false); settings.setValue("fUseProxy", false);
if (!settings.contains("addrProxy") || !settings.value("addrProxy").toString().contains(':')) if (!settings.contains("addrProxy"))
settings.setValue("addrProxy", "127.0.0.1:9050"); settings.setValue("addrProxy", QString("%1:%2").arg(DEFAULT_GUI_PROXY_HOST, DEFAULT_GUI_PROXY_PORT));
// Only try to set -proxy, if user has enabled fUseProxy // Only try to set -proxy, if user has enabled fUseProxy
if (settings.value("fUseProxy").toBool() && !gArgs.SoftSetArg("-proxy", settings.value("addrProxy").toString().toStdString())) if (settings.value("fUseProxy").toBool() && !gArgs.SoftSetArg("-proxy", settings.value("addrProxy").toString().toStdString()))
addOverriddenOption("-proxy"); addOverriddenOption("-proxy");
@ -134,8 +136,8 @@ void OptionsModel::Init(bool resetSettings)
if (!settings.contains("fUseSeparateProxyTor")) if (!settings.contains("fUseSeparateProxyTor"))
settings.setValue("fUseSeparateProxyTor", false); settings.setValue("fUseSeparateProxyTor", false);
if (!settings.contains("addrSeparateProxyTor") || !settings.value("addrSeparateProxyTor").toString().contains(':')) if (!settings.contains("addrSeparateProxyTor"))
settings.setValue("addrSeparateProxyTor", "127.0.0.1:9050"); settings.setValue("addrSeparateProxyTor", QString("%1:%2").arg(DEFAULT_GUI_PROXY_HOST, DEFAULT_GUI_PROXY_PORT));
// Only try to set -onion, if user has enabled fUseSeparateProxyTor // Only try to set -onion, if user has enabled fUseSeparateProxyTor
if (settings.value("fUseSeparateProxyTor").toBool() && !gArgs.SoftSetArg("-onion", settings.value("addrSeparateProxyTor").toString().toStdString())) if (settings.value("fUseSeparateProxyTor").toBool() && !gArgs.SoftSetArg("-onion", settings.value("addrSeparateProxyTor").toString().toStdString()))
addOverriddenOption("-onion"); addOverriddenOption("-onion");
@ -200,6 +202,33 @@ int OptionsModel::rowCount(const QModelIndex & parent) const
return OptionIDRowCount; return OptionIDRowCount;
} }
struct ProxySetting {
bool is_set;
QString ip;
QString port;
};
static ProxySetting GetProxySetting(QSettings &settings, const QString &name)
{
static const ProxySetting default_val = {false, DEFAULT_GUI_PROXY_HOST, QString("%1").arg(DEFAULT_GUI_PROXY_PORT)};
// Handle the case that the setting is not set at all
if (!settings.contains(name)) {
return default_val;
}
// contains IP at index 0 and port at index 1
QStringList ip_port = settings.value(name).toString().split(":", QString::SkipEmptyParts);
if (ip_port.size() == 2) {
return {true, ip_port.at(0), ip_port.at(1)};
} else { // Invalid: return default
return default_val;
}
}
static void SetProxySetting(QSettings &settings, const QString &name, const ProxySetting &ip_port)
{
settings.setValue(name, ip_port.ip + ":" + ip_port.port);
}
// read QSettings values and return them // read QSettings values and return them
QVariant OptionsModel::data(const QModelIndex & index, int role) const QVariant OptionsModel::data(const QModelIndex & index, int role) const
{ {
@ -226,30 +255,18 @@ QVariant OptionsModel::data(const QModelIndex & index, int role) const
// default proxy // default proxy
case ProxyUse: case ProxyUse:
return settings.value("fUseProxy", false); return settings.value("fUseProxy", false);
case ProxyIP: { case ProxyIP:
// contains IP at index 0 and port at index 1 return GetProxySetting(settings, "addrProxy").ip;
QStringList strlIpPort = settings.value("addrProxy").toString().split(":", QString::SkipEmptyParts); case ProxyPort:
return strlIpPort.at(0); return GetProxySetting(settings, "addrProxy").port;
}
case ProxyPort: {
// contains IP at index 0 and port at index 1
QStringList strlIpPort = settings.value("addrProxy").toString().split(":", QString::SkipEmptyParts);
return strlIpPort.at(1);
}
// separate Tor proxy // separate Tor proxy
case ProxyUseTor: case ProxyUseTor:
return settings.value("fUseSeparateProxyTor", false); return settings.value("fUseSeparateProxyTor", false);
case ProxyIPTor: { case ProxyIPTor:
// contains IP at index 0 and port at index 1 return GetProxySetting(settings, "addrSeparateProxyTor").ip;
QStringList strlIpPort = settings.value("addrSeparateProxyTor").toString().split(":", QString::SkipEmptyParts); case ProxyPortTor:
return strlIpPort.at(0); return GetProxySetting(settings, "addrSeparateProxyTor").port;
}
case ProxyPortTor: {
// contains IP at index 0 and port at index 1
QStringList strlIpPort = settings.value("addrSeparateProxyTor").toString().split(":", QString::SkipEmptyParts);
return strlIpPort.at(1);
}
#ifdef ENABLE_WALLET #ifdef ENABLE_WALLET
case SpendZeroConfChange: case SpendZeroConfChange:
@ -314,25 +331,19 @@ bool OptionsModel::setData(const QModelIndex & index, const QVariant & value, in
} }
break; break;
case ProxyIP: { case ProxyIP: {
// contains current IP at index 0 and current port at index 1 auto ip_port = GetProxySetting(settings, "addrProxy");
QStringList strlIpPort = settings.value("addrProxy").toString().split(":", QString::SkipEmptyParts); if (!ip_port.is_set || ip_port.ip != value.toString()) {
// if that key doesn't exist or has a changed IP ip_port.ip = value.toString();
if (!settings.contains("addrProxy") || strlIpPort.at(0) != value.toString()) { SetProxySetting(settings, "addrProxy", ip_port);
// construct new value from new IP and current port
QString strNewValue = value.toString() + ":" + strlIpPort.at(1);
settings.setValue("addrProxy", strNewValue);
setRestartRequired(true); setRestartRequired(true);
} }
} }
break; break;
case ProxyPort: { case ProxyPort: {
// contains current IP at index 0 and current port at index 1 auto ip_port = GetProxySetting(settings, "addrProxy");
QStringList strlIpPort = settings.value("addrProxy").toString().split(":", QString::SkipEmptyParts); if (!ip_port.is_set || ip_port.port != value.toString()) {
// if that key doesn't exist or has a changed port ip_port.port = value.toString();
if (!settings.contains("addrProxy") || strlIpPort.at(1) != value.toString()) { SetProxySetting(settings, "addrProxy", ip_port);
// construct new value from current IP and new port
QString strNewValue = strlIpPort.at(0) + ":" + value.toString();
settings.setValue("addrProxy", strNewValue);
setRestartRequired(true); setRestartRequired(true);
} }
} }
@ -346,25 +357,19 @@ bool OptionsModel::setData(const QModelIndex & index, const QVariant & value, in
} }
break; break;
case ProxyIPTor: { case ProxyIPTor: {
// contains current IP at index 0 and current port at index 1 auto ip_port = GetProxySetting(settings, "addrSeparateProxyTor");
QStringList strlIpPort = settings.value("addrSeparateProxyTor").toString().split(":", QString::SkipEmptyParts); if (!ip_port.is_set || ip_port.ip != value.toString()) {
// if that key doesn't exist or has a changed IP ip_port.ip = value.toString();
if (!settings.contains("addrSeparateProxyTor") || strlIpPort.at(0) != value.toString()) { SetProxySetting(settings, "addrSeparateProxyTor", ip_port);
// construct new value from new IP and current port
QString strNewValue = value.toString() + ":" + strlIpPort.at(1);
settings.setValue("addrSeparateProxyTor", strNewValue);
setRestartRequired(true); setRestartRequired(true);
} }
} }
break; break;
case ProxyPortTor: { case ProxyPortTor: {
// contains current IP at index 0 and current port at index 1 auto ip_port = GetProxySetting(settings, "addrSeparateProxyTor");
QStringList strlIpPort = settings.value("addrSeparateProxyTor").toString().split(":", QString::SkipEmptyParts); if (!ip_port.is_set || ip_port.port != value.toString()) {
// if that key doesn't exist or has a changed port ip_port.port = value.toString();
if (!settings.contains("addrSeparateProxyTor") || strlIpPort.at(1) != value.toString()) { SetProxySetting(settings, "addrSeparateProxyTor", ip_port);
// construct new value from current IP and new port
QString strNewValue = strlIpPort.at(0) + ":" + value.toString();
settings.setValue("addrSeparateProxyTor", strNewValue);
setRestartRequired(true); setRestartRequired(true);
} }
} }

View File

@ -13,6 +13,9 @@ QT_BEGIN_NAMESPACE
class QNetworkProxy; class QNetworkProxy;
QT_END_NAMESPACE QT_END_NAMESPACE
extern const char *DEFAULT_GUI_PROXY_HOST;
static constexpr unsigned short DEFAULT_GUI_PROXY_PORT = 9050;
/** Interface from Qt to configuration data structure for Bitcoin client. /** Interface from Qt to configuration data structure for Bitcoin client.
To Qt, the options are presented as a list with the different options To Qt, the options are presented as a list with the different options
laid out vertically. laid out vertically.