Merge pull request #810 from flowln/error_on_bad_file_paths_as_we_should_catquake
This commit is contained in:
parent
b7e96bdf62
commit
68c884f20d
@ -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());
|
||||
|
@ -225,10 +225,19 @@ bool ModrinthCreationTask::createInstance()
|
||||
|
||||
m_files_job = 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();
|
||||
|
Loading…
Reference in New Issue
Block a user