0xf8/pollymc
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Merge pull request #810 from flowln/error_on_bad_file_paths_as_we_should_catquake

Browse Source
This commit is contained in:
flow 2023-02-02 13:47:36 -08:00 committed by GitHub
commit 3781c64d41
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 25 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
launcher/minecraft/OneSixVersionFormat.cpp
View File

@ -39,6 +39,8 @@
#include "minecraft/ParseUtils.h"
#include <minecraft/MojangVersionFormat.h>
#include <QRegularExpression>
using namespace Json;
static void readString(const QJsonObject &root, const QString &key, QString &variable)
@ -121,6 +123,15 @@ VersionFilePtr OneSixVersionFormat::versionFileFromJson(const QJsonDocument &doc
out->uid = root.value("fileId").toString();
}
const QRegularExpression valid_uid_regex{ QRegularExpression::anchoredPattern(QStringLiteral(R"(\w+(?:\.\w+)*)")) };
if (!valid_uid_regex.match(out->uid).hasMatch()) {
qCritical() << "The component's 'uid' contains illegal characters! UID:" << out->uid;
out->addProblem(
ProblemSeverity::Error,
QObject::tr("The component's 'uid' contains illegal characters! This can cause security issues.")
);
}
out->version = root.value("version").toString();
MojangVersionFormat::readVersionProperties(root, out.get());

19
launcher/modplatform/modrinth/ModrinthInstanceCreationTask.cpp
View File

@ -225,10 +225,19 @@ bool ModrinthCreationTask::createInstance()
m_files_job.reset(new NetJob(tr("Mod download"), APPLICATION->network()));
auto root_modpack_path = FS::PathCombine(m_stagingPath, ".minecraft");
auto root_modpack_url = QUrl::fromLocalFile(root_modpack_path);
for (auto file : m_files) {
auto path = FS::PathCombine(m_stagingPath, ".minecraft", file.path);
qDebug() << "Will try to download" << file.downloads.front() << "to" << path;
auto dl = Net::Download::makeFile(file.downloads.dequeue(), path);
auto file_path = FS::PathCombine(root_modpack_path, file.path);
if (!root_modpack_url.isParentOf(QUrl::fromLocalFile(file_path))) {
// This means we somehow got out of the root folder, so abort here to prevent exploits
setError(tr("One of the files has a path that leads to an arbitrary location (%1). This is a security risk and isn't allowed.").arg(file.path));
return false;
}
qDebug() << "Will try to download" << file.downloads.front() << "to" << file_path;
auto dl = Net::Download::makeFile(file.downloads.dequeue(), file_path);
dl->addValidator(new Net::ChecksumValidator(file.hashAlgorithm, file.hash));
m_files_job->addNetAction(dl);
@ -236,8 +245,8 @@ bool ModrinthCreationTask::createInstance()
// FIXME: This really needs to be put into a ConcurrentTask of
// MultipleOptionsTask's , once those exist :)
auto param = dl.toWeakRef();
connect(dl.get(), &NetAction::failed, [this, &file, path, param] {
auto ndl = Net::Download::makeFile(file.downloads.dequeue(), path);
connect(dl.get(), &NetAction::failed, [this, &file, file_path, param] {
auto ndl = Net::Download::makeFile(file.downloads.dequeue(), file_path);
ndl->addValidator(new Net::ChecksumValidator(file.hashAlgorithm, file.hash));
m_files_job->addNetAction(ndl);
if (auto shared = param.lock()) shared->succeeded();