Browse Source

Fix empty string parameter was omitted

`QProcess::splitCommand()` will omit empty strings like `""` so provide
our own replacement.

Closes #13124.
adaptive-webui-19844
Chocobo1 2 years ago
parent
commit
0802b6d506
No known key found for this signature in database
GPG Key ID: 210D9C873253A68C
  1. 128
      src/app/application.cpp
  2. 37
      src/base/utils/string.cpp
  3. 2
      src/base/utils/string.h
  4. 1
      test/CMakeLists.txt
  5. 59
      test/testutilsstring.cpp

128
src/app/application.cpp

@ -83,6 +83,7 @@ @@ -83,6 +83,7 @@
#include "base/utils/compare.h"
#include "base/utils/fs.h"
#include "base/utils/misc.h"
#include "base/utils/string.h"
#include "base/version.h"
#include "applicationinstancemanager.h"
#include "filelogger.h"
@ -353,64 +354,74 @@ void Application::processMessage(const QString &message) @@ -353,64 +354,74 @@ void Application::processMessage(const QString &message)
void Application::runExternalProgram(const BitTorrent::Torrent *torrent) const
{
QString program = Preferences::instance()->getAutoRunProgram().trimmed();
// Cannot give users shell environment by default, as doing so could
// enable command injection via torrent name and other arguments
// (especially when some automated download mechanism has been setup).
// See: https://github.com/qbittorrent/qBittorrent/issues/10925
for (int i = (program.length() - 2); i >= 0; --i)
const auto replaceVariables = [torrent](QString str) -> QString
{
if (program[i] != u'%')
continue;
const ushort specifier = program[i + 1].unicode();
switch (specifier)
for (int i = (str.length() - 2); i >= 0; --i)
{
case u'C':
program.replace(i, 2, QString::number(torrent->filesCount()));
break;
case u'D':
program.replace(i, 2, torrent->savePath().toString());
break;
case u'F':
program.replace(i, 2, torrent->contentPath().toString());
break;
case u'G':
program.replace(i, 2, torrent->tags().join(u","_qs));
break;
case u'I':
program.replace(i, 2, (torrent->infoHash().v1().isValid() ? torrent->infoHash().v1().toString() : u"-"_qs));
break;
case u'J':
program.replace(i, 2, (torrent->infoHash().v2().isValid() ? torrent->infoHash().v2().toString() : u"-"_qs));
break;
case u'K':
program.replace(i, 2, torrent->id().toString());
break;
case u'L':
program.replace(i, 2, torrent->category());
break;
case u'N':
program.replace(i, 2, torrent->name());
break;
case u'R':
program.replace(i, 2, torrent->rootPath().toString());
break;
case u'T':
program.replace(i, 2, torrent->currentTracker());
break;
case u'Z':
program.replace(i, 2, QString::number(torrent->totalSize()));
break;
default:
// do nothing
break;
if (str[i] != u'%')
continue;
const ushort specifier = str[i + 1].unicode();
switch (specifier)
{
case u'C':
str.replace(i, 2, QString::number(torrent->filesCount()));
break;
case u'D':
str.replace(i, 2, torrent->savePath().toString());
break;
case u'F':
str.replace(i, 2, torrent->contentPath().toString());
break;
case u'G':
str.replace(i, 2, torrent->tags().join(u","_qs));
break;
case u'I':
str.replace(i, 2, (torrent->infoHash().v1().isValid() ? torrent->infoHash().v1().toString() : u"-"_qs));
break;
case u'J':
str.replace(i, 2, (torrent->infoHash().v2().isValid() ? torrent->infoHash().v2().toString() : u"-"_qs));
break;
case u'K':
str.replace(i, 2, torrent->id().toString());
break;
case u'L':
str.replace(i, 2, torrent->category());
break;
case u'N':
str.replace(i, 2, torrent->name());
break;
case u'R':
str.replace(i, 2, torrent->rootPath().toString());
break;
case u'T':
str.replace(i, 2, torrent->currentTracker());
break;
case u'Z':
str.replace(i, 2, QString::number(torrent->totalSize()));
break;
default:
// do nothing
break;
}
// decrement `i` to avoid unwanted replacement, example pattern: "%%N"
--i;
}
// decrement `i` to avoid unwanted replacement, example pattern: "%%N"
--i;
}
return str;
};
LogMsg(tr("Torrent: %1, running external program, command: %2").arg(torrent->name(), program));
const QString logMsg = tr("Running external program. Torrent: \"%1\". Command: `%2`");
// The processing sequenece is different for Windows and other OS, this is intentional
#if defined(Q_OS_WIN)
const QString program = replaceVariables(Preferences::instance()->getAutoRunProgram().trimmed());
const std::wstring programWStr = program.toStdWString();
// Need to split arguments manually because QProcess::startDetached(QString)
@ -419,10 +430,15 @@ void Application::runExternalProgram(const BitTorrent::Torrent *torrent) const @@ -419,10 +430,15 @@ void Application::runExternalProgram(const BitTorrent::Torrent *torrent) const
int argCount = 0;
std::unique_ptr<LPWSTR[], decltype(&::LocalFree)> args {::CommandLineToArgvW(programWStr.c_str(), &argCount), ::LocalFree};
if (argCount <= 0)
return;
QStringList argList;
for (int i = 1; i < argCount; ++i)
argList += QString::fromWCharArray(args[i]);
LogMsg(logMsg.arg(torrent->name(), program));
QProcess proc;
proc.setProgram(QString::fromWCharArray(args[0]));
proc.setArguments(argList);
@ -449,14 +465,16 @@ void Application::runExternalProgram(const BitTorrent::Torrent *torrent) const @@ -449,14 +465,16 @@ void Application::runExternalProgram(const BitTorrent::Torrent *torrent) const
});
proc.startDetached();
#else // Q_OS_WIN
// Cannot give users shell environment by default, as doing so could
// enable command injection via torrent name and other arguments
// (especially when some automated download mechanism has been setup).
// See: https://github.com/qbittorrent/qBittorrent/issues/10925
QStringList args = QProcess::splitCommand(program);
QStringList args = Utils::String::splitCommand(Preferences::instance()->getAutoRunProgram().trimmed());
if (args.isEmpty())
return;
for (QString &arg : args)
arg = replaceVariables(arg);
LogMsg(logMsg.arg(torrent->name(), args.join(u' ')));
const QString command = args.takeFirst();
QProcess::startDetached(command, args);
#endif

37
src/base/utils/string.cpp

@ -33,6 +33,7 @@ @@ -33,6 +33,7 @@
#include <QLocale>
#include <QRegularExpression>
#include <QStringList>
#include <QVector>
#if (QT_VERSION < QT_VERSION_CHECK(6, 0, 0))
@ -76,6 +77,42 @@ QString Utils::String::wildcardToRegexPattern(const QString &pattern) @@ -76,6 +77,42 @@ QString Utils::String::wildcardToRegexPattern(const QString &pattern)
}
#endif
QStringList Utils::String::splitCommand(const QString &command)
{
QStringList ret;
ret.reserve(32);
bool inQuotes = false;
QString tmp;
for (const QChar c : command)
{
if (c == u' ')
{
if (!inQuotes)
{
if (!tmp.isEmpty())
{
ret.append(tmp);
tmp.clear();
}
continue;
}
}
else if (c == u'"')
{
inQuotes = !inQuotes;
}
tmp.append(c);
}
if (!tmp.isEmpty())
ret.append(tmp);
return ret;
}
std::optional<bool> Utils::String::parseBool(const QString &string)
{
if (string.compare(u"true", Qt::CaseInsensitive) == 0)

2
src/base/utils/string.h

@ -61,6 +61,8 @@ namespace Utils::String @@ -61,6 +61,8 @@ namespace Utils::String
std::optional<int> parseInt(const QString &string);
std::optional<double> parseDouble(const QString &string);
QStringList splitCommand(const QString &command);
QString join(const QList<QStringView> &strings, QStringView separator);
QString fromDouble(double n, int precision);

1
test/CMakeLists.txt

@ -14,6 +14,7 @@ set(testFiles @@ -14,6 +14,7 @@ set(testFiles
testorderedset.cpp
testutilscompare.cpp
testutilsgzip.cpp
testutilsstring.cpp
)
foreach(testFile ${testFiles})
get_filename_component(testFilename "${testFile}" NAME_WLE)

59
test/testutilsstring.cpp

@ -0,0 +1,59 @@ @@ -0,0 +1,59 @@
/*
* Bittorrent Client using Qt and libtorrent.
* Copyright (C) 2022 Mike Tzou (Chocobo1)
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU General Public License
* as published by the Free Software Foundation; either version 2
* of the License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program; if not, write to the Free Software
* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
*
* In addition, as a special exception, the copyright holders give permission to
* link this program with the OpenSSL project's "OpenSSL" library (or with
* modified versions of it that use the same license as the "OpenSSL" library),
* and distribute the linked executables. You must obey the GNU General Public
* License in all respects for all of the code used other than "OpenSSL". If you
* modify file(s), you may extend this exception to your version of the file(s),
* but you are not obligated to do so. If you do not wish to do so, delete this
* exception statement from your version.
*/
#include <QTest>
#include "base/global.h"
#include "base/utils/string.h"
class TestUtilsString final : public QObject
{
Q_OBJECT
Q_DISABLE_COPY_MOVE(TestUtilsString)
public:
TestUtilsString() = default;
private slots:
void testSplitCommand() const
{
QCOMPARE(Utils::String::splitCommand({}), {});
QCOMPARE(Utils::String::splitCommand(u""_qs), {});
QCOMPARE(Utils::String::splitCommand(u" "_qs), {});
QCOMPARE(Utils::String::splitCommand(uR"("")"_qs), {uR"("")"_qs});
QCOMPARE(Utils::String::splitCommand(uR"(" ")"_qs), {uR"(" ")"_qs});
QCOMPARE(Utils::String::splitCommand(u"\"\"\""_qs), {u"\"\"\""_qs});
QCOMPARE(Utils::String::splitCommand(uR"(" """)"_qs), {uR"(" """)"_qs});
QCOMPARE(Utils::String::splitCommand(u" app a b c "_qs), QStringList({u"app"_qs, u"a"_qs, u"b"_qs, u"c"_qs}));
QCOMPARE(Utils::String::splitCommand(u" cmd.exe /d --arg2 \"arg3\" \"\" arg5 \"\"arg6 \"arg7 "_qs)
, QStringList({u"cmd.exe"_qs, u"/d"_qs, u"--arg2"_qs, u"\"arg3\""_qs, u"\"\""_qs, u"arg5"_qs, u"\"\"arg6"_qs, u"\"arg7 "_qs}));
}
};
QTEST_APPLESS_MAIN(TestUtilsString)
#include "testutilsstring.moc"
Loading…
Cancel
Save