Skip to content
Permalink
Browse files Browse the repository at this point in the history
FIX(client): Only allow "http"/"https" for URLs in ConnectDialog
Our public server list registration script doesn't have an URL scheme
whitelist for the website field.

Turns out a malicious server can register itself with a dangerous URL in
an attempt to attack a user's machine.

User interaction is required, as the URL has to be opened by
right-clicking on the server entry and clicking on "Open Webpage".

This commit introduces a client-side whitelist, which only allows "http"
and "https" schemes. We will also implement it in our public list.

In future we should probably add a warning QMessageBox informing the
user that there's no guarantee the URL is safe (regardless of the
scheme).

Thanks a lot to https://positive.security for reporting the RCE
vulnerability to us privately.
  • Loading branch information
davidebeatrici authored and Krzmbrzl committed Feb 6, 2021
1 parent 6efbd83 commit e59ee87
Showing 1 changed file with 17 additions and 3 deletions.
20 changes: 17 additions & 3 deletions src/mumble/ConnectDialog.cpp
Expand Up @@ -1265,11 +1265,25 @@ void ConnectDialog::on_qaFavoritePaste_triggered() {
}

void ConnectDialog::on_qaUrl_triggered() {
ServerItem *si = static_cast<ServerItem *>(qtwServers->currentItem());
if (! si || si->qsUrl.isEmpty())
auto *si = static_cast< const ServerItem * >(qtwServers->currentItem());
if (!si || si->qsUrl.isEmpty()) {
return;
}

const QStringList allowedSchemes = { QLatin1String("http"), QLatin1String("https") };

QDesktopServices::openUrl(QUrl(si->qsUrl));
const auto url = QUrl(si->qsUrl);
if (allowedSchemes.contains(url.scheme())) {
QDesktopServices::openUrl(url);
} else {
// Inform user that the requested URL has been blocked
QMessageBox msgBox;
msgBox.setText(QObject::tr("<b>Blocked URL scheme \"%1\"</b>").arg(url.scheme()));
msgBox.setInformativeText(QObject::tr("The URL uses a scheme that has been blocked for security reasons."));
msgBox.setDetailedText(QObject::tr("Blocked URL: \"%1\"").arg(url.toString()));
msgBox.setIcon(QMessageBox::Warning);
msgBox.exec();
}
}

void ConnectDialog::onFiltersTriggered(QAction *act) {
Expand Down

0 comments on commit e59ee87

Please sign in to comment.