aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--.github/workflows/build.yml25
-rw-r--r--launcher/minecraft/OneSixVersionFormat.cpp11
-rw-r--r--launcher/modplatform/modrinth/ModrinthInstanceCreationTask.cpp19
-rw-r--r--tests/Task_test.cpp30
4 files changed, 63 insertions, 22 deletions
diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml
index 6ec4f6a4..86e88fa1 100644
--- a/.github/workflows/build.yml
+++ b/.github/workflows/build.yml
@@ -149,6 +149,15 @@ jobs:
with:
key: ${{ matrix.os }}-qt${{ matrix.qt_ver }}-${{ matrix.architecture }}
+ - name: Retrieve ccache cache (Windows MinGW-w64)
+ if: runner.os == 'Windows' && matrix.msystem != '' && inputs.build_type == 'Debug'
+ uses: actions/cache@v3.2.4
+ with:
+ path: '${{ github.workspace }}\.ccache'
+ key: ${{ matrix.os }}-mingw-w64-ccache-${{ github.run_id }}
+ restore-keys: |
+ ${{ matrix.os }}-mingw-w64-ccache
+
- name: Setup ccache (Windows MinGW-w64)
if: runner.os == 'Windows' && matrix.msystem != '' && inputs.build_type == 'Debug'
shell: msys2 {0}
@@ -165,15 +174,6 @@ jobs:
run: |
echo "CCACHE_VAR=ccache" >> $GITHUB_ENV
- - name: Retrieve ccache cache (Windows MinGW-w64)
- if: runner.os == 'Windows' && matrix.msystem != '' && inputs.build_type == 'Debug'
- uses: actions/cache@v3.2.4
- with:
- path: '${{ github.workspace }}\.ccache'
- key: ${{ matrix.os }}-mingw-w64
- restore-keys: |
- ${{ matrix.os }}-mingw-w64
-
- name: Set short version
shell: bash
run: |
@@ -510,6 +510,13 @@ jobs:
with:
name: PrismLauncher-${{ runner.os }}-${{ env.VERSION }}-${{ inputs.build_type }}-x86_64.AppImage
path: PrismLauncher-${{ runner.os }}-${{ env.VERSION }}-${{ inputs.build_type }}-x86_64.AppImage
+
+ - name: ccache stats (Windows MinGW-w64)
+ if: runner.os == 'Windows' && matrix.msystem != ''
+ shell: msys2 {0}
+ run: |
+ ccache -s
+
snap:
runs-on: ubuntu-20.04
steps:
diff --git a/launcher/minecraft/OneSixVersionFormat.cpp b/launcher/minecraft/OneSixVersionFormat.cpp
index 280f6b26..888b6860 100644
--- a/launcher/minecraft/OneSixVersionFormat.cpp
+++ b/launcher/minecraft/OneSixVersionFormat.cpp
@@ -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"([a-zA-Z0-9-_]+(?:\.[a-zA-Z0-9-_]+)*)")) };
+ 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());
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();
diff --git a/tests/Task_test.cpp b/tests/Task_test.cpp
index 558cd2c0..95eb4a30 100644
--- a/tests/Task_test.cpp
+++ b/tests/Task_test.cpp
@@ -7,6 +7,8 @@
#include <tasks/SequentialTask.h>
#include <tasks/Task.h>
+#include <array>
+
/* Does nothing. Only used for testing. */
class BasicTask : public Task {
Q_OBJECT
@@ -35,10 +37,23 @@ class BasicTask_MultiStep : public Task {
void executeTask() override {};
};
-class BigConcurrentTask : public QThread {
+class BigConcurrentTask : public ConcurrentTask {
+ Q_OBJECT
+
+ void startNext() override
+ {
+ // This is here only to help fill the stack a bit more quickly (if there's an issue, of course :^))
+ // Each tasks thus adds 1024 * 4 bytes to the stack, at the very least.
+ [[maybe_unused]] volatile std::array<uint32_t, 1024> some_data_on_the_stack {};
+
+ ConcurrentTask::startNext();
+ }
+};
+
+class BigConcurrentTaskThread : public QThread {
Q_OBJECT
- ConcurrentTask big_task;
+ BigConcurrentTask big_task;
void run() override
{
@@ -48,7 +63,9 @@ class BigConcurrentTask : public QThread {
deadline.start();
// NOTE: Arbitrary value that manages to trigger a problem when there is one.
- static const unsigned s_num_tasks = 1 << 14;
+ // Considering each tasks, in a problematic state, adds 1024 * 4 bytes to the stack,
+ // this number is enough to fill up 16 MiB of stack, more than enough to cause a problem.
+ static const unsigned s_num_tasks = 1 << 12;
auto sub_tasks = new BasicTask::Ptr[s_num_tasks];
for (unsigned i = 0; i < s_num_tasks; i++) {
@@ -237,12 +254,9 @@ class TaskTest : public QObject {
{
QEventLoop loop;
- auto thread = new BigConcurrentTask;
- // NOTE: This is an arbitrary value, big enough to not cause problems on normal execution, but low enough
- // so that the number of tasks that needs to get ran to potentially cause a problem isn't too big.
- thread->setStackSize(32 * 1024);
+ auto thread = new BigConcurrentTaskThread;
- connect(thread, &BigConcurrentTask::finished, &loop, &QEventLoop::quit);
+ connect(thread, &BigConcurrentTaskThread::finished, &loop, &QEventLoop::quit);
thread->start();