From 435273e08a3cf6cb8197acabb31b1d4889a87254 Mon Sep 17 00:00:00 2001 From: flow Date: Tue, 31 Jan 2023 10:28:39 -0300 Subject: [PATCH] fix(Inst.Import): don't allow bad file path in mrpack import This checks the URL of the path of the file to be downloaded, ensuring that it always contains the root .minecraft target folder, following the warning in the mrpack documentation. Signed-off-by: flow --- .../modrinth/ModrinthInstanceCreationTask.cpp | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/launcher/modplatform/modrinth/ModrinthInstanceCreationTask.cpp b/launcher/modplatform/modrinth/ModrinthInstanceCreationTask.cpp index 94c0bf77..6814e645 100644 --- a/launcher/modplatform/modrinth/ModrinthInstanceCreationTask.cpp +++ b/launcher/modplatform/modrinth/ModrinthInstanceCreationTask.cpp @@ -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();