CVE-2017-6503: Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting') (CWE-79)
value: WebUI in qBittorrent before 3.3.11 did not escape many values, which could potentially lead to XSS.
CWE description: The product does not neutralize or incorrectly neutralizes user-controllable input before it is placed in output that is used as a web page that is served to other users.
CVSS v3 base score: 6.1
Published: 2017-03-06T02:59Z
Repository: https://github.com/qbittorrent/qBittorrent
Language: C++
Files changed: src/base/logger.cpp, src/base/utils/string.cpp, src/base/utils/string.h, src/gui/deletionconfirmationdlg.h, src/gui/properties/peerlistwidget.cpp, src/gui/properties/propertieswidget.cpp
Commit date: 2017-03-03 21:28:30 +0200
Commit message: Add Utils::String::toHtmlEscaped

Diff: diff --git a/src/base/logger.cpp b/src/base/logger.cpp
index d0a1e1220..d2e95ab58 100644
--- a/src/base/logger.cpp
+++ b/src/base/logger.cpp
@@ -1,8 +1,9 @@
 #include "logger.h"
 
 #include <QDateTime>
+#include "base/utils/string.h"
 
 Logger* Logger::m_instance = 0;
 
 Logger::Logger()
     : lock(QReadWriteLock::Recursive)
@@ -34,11 +35,11 @@ void Logger::freeInstance()
 
 void Logger::addMessage(const QString &message, const Log::MsgType &type)
 {
     QWriteLocker locker(&lock);
 
-    Log::Msg temp = { msgCounter++, QDateTime::currentMSecsSinceEpoch(), type, message };
+    Log::Msg temp = { msgCounter++, QDateTime::currentMSecsSinceEpoch(), type, Utils::String::toHtmlEscaped(message) };
     m_messages.push_back(temp);
 
     if (m_messages.size() >= MAX_LOG_MESSAGES)
         m_messages.pop_front();
 
@@ -47,11 +48,11 @@ void Logger::addMessage(const QString &message, const Log::MsgType &type)
 
 void Logger::addPeer(const QString &ip, bool blocked, const QString &reason)
 {
     QWriteLocker locker(&lock);
 
-    Log::Peer temp = { peerCounter++, QDateTime::currentMSecsSinceEpoch(), ip, blocked, reason };
+    Log::Peer temp = { peerCounter++, QDateTime::currentMSecsSinceEpoch(), Utils::String::toHtmlEscaped(ip), blocked, Utils::String::toHtmlEscaped(reason) };
     m_peers.push_back(temp);
 
     if (m_peers.size() >= MAX_LOG_MESSAGES)
         m_peers.pop_front();
 
diff --git a/src/base/utils/string.cpp b/src/base/utils/string.cpp
index 51e4d62ac..7d76d9aaf 100644
--- a/src/base/utils/string.cpp
+++ b/src/base/utils/string.cpp
@@ -209,5 +209,14 @@ bool Utils::String::slowEquals(const QByteArray &a, const QByteArray &b)
     for (int i = 0; (i < lengthA) && (i < lengthB); i++)
         diff |= a[i] ^ b[i];
 
     return (diff == 0);
 }
+
+QString Utils::String::toHtmlEscaped(const QString &str)
+{
+#ifdef QBT_USES_QT5
+    return str.toHtmlEscaped();
+#else
+    return Qt::escape(str);
+#endif
+}
diff --git a/src/base/utils/string.h b/src/base/utils/string.h
index 8ca2f2e45..52f4863ef 100644
--- a/src/base/utils/string.h
+++ b/src/base/utils/string.h
@@ -45,10 +45,12 @@ namespace Utils
 
         // Implements constant-time comparison to protect against timing attacks
         // Taken from https://crackstation.net/hashing-security.htm
         bool slowEquals(const QByteArray &a, const QByteArray &b);
 
+        QString toHtmlEscaped(const QString &str);
+
         bool naturalCompareCaseSensitive(const QString &left, const QString &right);
         bool naturalCompareCaseInsensitive(const QString &left, const QString &right);
     }
 }
 
diff --git a/src/gui/deletionconfirmationdlg.h b/src/gui/deletionconfirmationdlg.h
index 2715f7ce9..7e65df46c 100644
--- a/src/gui/deletionconfirmationdlg.h
+++ b/src/gui/deletionconfirmationdlg.h
@@ -33,21 +33,22 @@
 
 #include <QDialog>
 #include <QPushButton>
 #include "ui_confirmdeletiondlg.h"
 #include "base/preferences.h"
-#include "guiiconprovider.h"
 #include "base/utils/misc.h"
+#include "base/utils/string.h"
+#include "guiiconprovider.h"
 
 class DeletionConfirmationDlg : public QDialog, private Ui::confirmDeletionDlg {
   Q_OBJECT
 
   public:
   DeletionConfirmationDlg(QWidget *parent, const int &size, const QString &name, bool defaultDeleteFiles): QDialog(parent) {
     setupUi(this);
     if (size == 1)
-      label->setText(tr("Are you sure you want to delete '%1' from the transfer list?", "Are you sure you want to delete 'ubuntu-linux-iso' from the transfer list?").arg(name));
+      label->setText(tr("Are you sure you want to delete '%1' from the transfer list?", "Are you sure you want to delete 'ubuntu-linux-iso' from the transfer list?").arg(Utils::String::toHtmlEscaped(name)));
     else
       label->setText(tr("Are you sure you want to delete these %1 torrents from the transfer list?", "Are you sure you want to delete these 5 torrents from the transfer list?").arg(QString::number(size)));
     // Icons
     lbl_warn->setPixmap(GuiIconProvider::instance()->getIcon("dialog-warning").pixmap(lbl_warn->height()));
     lbl_warn->setFixedWidth(lbl_warn->height());
diff --git a/src/gui/properties/peerlistwidget.cpp b/src/gui/properties/peerlistwidget.cpp
index eb6a49d7c..01551e12c 100644
--- a/src/gui/properties/peerlistwidget.cpp
+++ b/src/gui/properties/peerlistwidget.cpp
@@ -399,11 +399,11 @@ QStandardItem* PeerListWidget::addPeer(const QString& ip, BitTorrent::TorrentHan
         }
     }
     m_listModel->setData(m_listModel->index(row, PeerListDelegate::CONNECTION), peer.connectionType());
     m_listModel->setData(m_listModel->index(row, PeerListDelegate::FLAGS), peer.flags());
     m_listModel->setData(m_listModel->index(row, PeerListDelegate::FLAGS), peer.flagsDescription(), Qt::ToolTipRole);
-    m_listModel->setData(m_listModel->index(row, PeerListDelegate::CLIENT), peer.client());
+    m_listModel->setData(m_listModel->index(row, PeerListDelegate::CLIENT), Utils::String::toHtmlEscaped(peer.client()));
     m_listModel->setData(m_listModel->index(row, PeerListDelegate::PROGRESS), peer.progress());
     m_listModel->setData(m_listModel->index(row, PeerListDelegate::DOWN_SPEED), peer.payloadDownSpeed());
     m_listModel->setData(m_listModel->index(row, PeerListDelegate::UP_SPEED), peer.payloadUpSpeed());
     m_listModel->setData(m_listModel->index(row, PeerListDelegate::TOT_DOWN), peer.totalDownload());
     m_listModel->setData(m_listModel->index(row, PeerListDelegate::TOT_UP), peer.totalUpload());
@@ -430,11 +430,11 @@ void PeerListWidget::updatePeer(const QString &ip, BitTorrent::TorrentHandle *co
     }
     m_listModel->setData(m_listModel->index(row, PeerListDelegate::CONNECTION), peer.connectionType());
     m_listModel->setData(m_listModel->index(row, PeerListDelegate::PORT), peer.address().port);
     m_listModel->setData(m_listModel->index(row, PeerListDelegate::FLAGS), peer.flags());
     m_listModel->setData(m_listModel->index(row, PeerListDelegate::FLAGS), peer.flagsDescription(), Qt::ToolTipRole);
-    m_listModel->setData(m_listModel->index(row, PeerListDelegate::CLIENT), peer.client());
+    m_listModel->setData(m_listModel->index(row, PeerListDelegate::CLIENT), Utils::String::toHtmlEscaped(peer.client()));
     m_listModel->setData(m_listModel->index(row, PeerListDelegate::PROGRESS), peer.progress());
     m_listModel->setData(m_listModel->index(row, PeerListDelegate::DOWN_SPEED), peer.payloadDownSpeed());
     m_listModel->setData(m_listModel->index(row, PeerListDelegate::UP_SPEED), peer.payloadUpSpeed());
     m_listModel->setData(m_listModel->index(row, PeerListDelegate::TOT_DOWN), peer.totalDownload());
     m_listModel->setData(m_listModel->index(row, PeerListDelegate::TOT_UP), peer.totalUpload());
diff --git a/src/gui/properties/propertieswidget.cpp b/src/gui/properties/propertieswidget.cpp
index 2dc3ee015..0e27c1c04 100644
--- a/src/gui/properties/propertieswidget.cpp
+++ b/src/gui/properties/propertieswidget.cpp
@@ -312,16 +312,16 @@ void PropertiesWidget::loadTorrentInfos(BitTorrent::TorrentHandle *const torrent
         lbl_creationDate->setText(m_torrent->creationDate().toString(Qt::DefaultLocaleShortDate));
 
         label_total_size_val->setText(Utils::Misc::friendlyUnit(m_torrent->totalSize()));
 
         // Comment
-        comment_text->setText(Utils::Misc::parseHtmlLinks(m_torrent->comment()));
+        comment_text->setText(Utils::Misc::parseHtmlLinks(Utils::String::toHtmlEscaped(m_torrent->comment())));
 
         // URL seeds
         loadUrlSeeds();
 
-        label_created_by_val->setText(m_torrent->creator());
+        label_created_by_val->setText(Utils::String::toHtmlEscaped(m_torrent->creator()));
 
         // List files in torrent
         PropListModel->model()->setupModelData(m_torrent->info());
         filesList->setExpanded(PropListModel->index(0, 0), true);
 

Vulnerable code:     Log::Msg temp = { msgCounter++, QDateTime::currentMSecsSinceEpoch(), type, message };
    Log::Peer temp = { peerCounter++, QDateTime::currentMSecsSinceEpoch(), ip, blocked, reason };
#include "guiiconprovider.h"
      label->setText(tr("Are you sure you want to delete '%1' from the transfer list?", "Are you sure you want to delete 'ubuntu-linux-iso' from the transfer list?").arg(name));
    m_listModel->setData(m_listModel->index(row, PeerListDelegate::CLIENT), peer.client());
    m_listModel->setData(m_listModel->index(row, PeerListDelegate::CLIENT), peer.client());
        comment_text->setText(Utils::Misc::parseHtmlLinks(m_torrent->comment()));
        label_created_by_val->setText(m_torrent->creator());
Fixed code: #include "base/utils/string.h"
    Log::Msg temp = { msgCounter++, QDateTime::currentMSecsSinceEpoch(), type, Utils::String::toHtmlEscaped(message) };
    Log::Peer temp = { peerCounter++, QDateTime::currentMSecsSinceEpoch(), Utils::String::toHtmlEscaped(ip), blocked, Utils::String::toHtmlEscaped(reason) };

QString Utils::String::toHtmlEscaped(const QString &str)
{
#ifdef QBT_USES_QT5
    return str.toHtmlEscaped();
#else
    return Qt::escape(str);
#endif
}
        QString toHtmlEscaped(const QString &str);

#include "base/utils/string.h"
#include "guiiconprovider.h"
      label->setText(tr("Are you sure you want to delete '%1' from the transfer list?", "Are you sure you want to delete 'ubuntu-linux-iso' from the transfer list?").arg(Utils::String::toHtmlEscaped(name)));
    m_listModel->setData(m_listModel->index(row, PeerListDelegate::CLIENT), Utils::String::toHtmlEscaped(peer.client()));
    m_listModel->setData(m_listModel->index(row, PeerListDelegate::CLIENT), Utils::String::toHtmlEscaped(peer.client()));
        comment_text->setText(Utils::Misc::parseHtmlLinks(Utils::String::toHtmlEscaped(m_torrent->comment())));
        label_created_by_val->setText(Utils::String::toHtmlEscaped(m_torrent->creator()));