fix heap overflow with unstable version comparation

fixes #5210
fixes #5251 (the removeDuplicates line)

The issue was mostly with the Version parsing and compring
implementation.
Refactored that based on the https://git.sleeping.town/exa/FlexVer
examples.

Signed-off-by: Trial97 <alexandru.tripon97@gmail.com>

launcher/Version.cpp

@@ -1,111 +1,28 @@
+// SPDX-License-Identifier: GPL-3.0-only
+/*
+ *  Prism Launcher - Minecraft Launcher
+ *  Copyright (C) 2023 flowln <flowlnlnln@gmail.com>
+ *  Copyright (C) 2022 Sefa Eyeoglu <contact@scrumplex.net>
+ *  Copyright (c) 2026 Trial97 <alexandru.tripon97@gmail.com>
+ *
+ *  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, version 3.
+ *
+ *  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, see <https://www.gnu.org/licenses/>.
+ */
 #include "Version.h"
 
 #include <QDebug>
 #include <QRegularExpressionMatch>
 #include <QUrl>
-
-Version::Version(QString str) : m_string(std::move(str))
-{
-    parse();
-}
-
-#define VERSION_OPERATOR(return_on_different)                                               \
-    bool exclude_our_sections = false;                                                      \
-    bool exclude_their_sections = false;                                                    \
-                                                                                            \
-    const auto size = qMax(m_sections.size(), other.m_sections.size());                     \
-    for (int i = 0; i < size; ++i) {                                                        \
-        Section sec1 = (i >= m_sections.size()) ? Section() : m_sections.at(i);             \
-        Section sec2 = (i >= other.m_sections.size()) ? Section() : other.m_sections.at(i); \
-                                                                                            \
-        { /* Don't include appendixes in the comparison */                                  \
-            if (sec1.isAppendix())                                                          \
-                exclude_our_sections = true;                                                \
-            if (sec2.isAppendix())                                                          \
-                exclude_their_sections = true;                                              \
-                                                                                            \
-            if (exclude_our_sections) {                                                     \
-                sec1 = Section();                                                           \
-                if (sec2.m_isNull)                                                          \
-                    break;                                                                  \
-            }                                                                               \
-                                                                                            \
-            if (exclude_their_sections) {                                                   \
-                sec2 = Section();                                                           \
-                if (sec1.m_isNull)                                                          \
-                    break;                                                                  \
-            }                                                                               \
-        }                                                                                   \
-                                                                                            \
-        if (sec1 != sec2)                                                                   \
-            return return_on_different;                                                     \
-    }
-
-bool Version::operator<(const Version& other) const
-{
-    VERSION_OPERATOR(sec1 < sec2)
-
-    return false;
-}
-bool Version::operator==(const Version& other) const
-{
-    VERSION_OPERATOR(false)
-
-    return true;
-}
-bool Version::operator!=(const Version& other) const
-{
-    return !operator==(other);
-}
-bool Version::operator<=(const Version& other) const
-{
-    return *this < other || *this == other;
-}
-bool Version::operator>(const Version& other) const
-{
-    return !(*this <= other);
-}
-bool Version::operator>=(const Version& other) const
-{
-    return !(*this < other);
-}
-
-void Version::parse()
-{
-    m_sections.clear();
-    QString currentSection;
-
-    if (m_string.isEmpty())
-        return;
-
-    auto classChange = [&currentSection](QChar lastChar, QChar currentChar) {
-        if (lastChar.isNull())
-            return false;
-        if (lastChar.isDigit() != currentChar.isDigit())
-            return true;
-
-        const QList<QChar> s_separators{ '.', '-', '+' };
-        if (s_separators.contains(currentChar) && currentSection.at(0) != currentChar)
-            return true;
-
-        return false;
-    };
-
-    currentSection += m_string.at(0);
-    for (int i = 1; i < m_string.size(); ++i) {
-        const auto& current_char = m_string.at(i);
-        if (classChange(m_string.at(i - 1), current_char)) {
-            if (!currentSection.isEmpty())
-                m_sections.append(Section(currentSection));
-            currentSection = "";
-        }
-
-        currentSection += current_char;
-    }
-
-    if (!currentSection.isEmpty())
-        m_sections.append(Section(currentSection));
-}
+#include <compare>
 
 /// qDebug print support for the Version class
 QDebug operator<<(QDebug debug, const Version& v)
@@ -115,10 +32,11 @@ QDebug operator<<(QDebug debug, const Version& v)
     debug.nospace() << "Version{ string: " << v.toString() << ", sections: [ ";
 
     bool first = true;
-    for (auto s : v.m_sections) {
-        if (!first)
+    for (const auto& s : v.m_sections) {
+        if (!first) {
             debug.nospace() << ", ";
-        debug.nospace() << s.m_fullString;
+        }
+        debug.nospace() << s.value;
         first = false;
     }
 
@@ -126,3 +44,111 @@ QDebug operator<<(QDebug debug, const Version& v)
 
     return debug;
 }
+
+std::strong_ordering Version::Section::operator<=>(const Section& other) const
+{
+    // If both components are numeric, compare numerically (codepoint-wise)
+    if (this->t == Type::Numeric && other.t == Type::Numeric) {
+        auto aLen = this->value.size();
+        if (aLen != other.value.size()) {
+            // Lengths differ; compare by length
+            return aLen <=> other.value.size();
+        }
+        // Compare by digits
+        auto cmp = QString::compare(this->value, other.value);
+        if (cmp < 0) {
+            return std::strong_ordering::less;
+        }
+        if (cmp > 0) {
+            return std::strong_ordering::greater;
+        }
+        return std::strong_ordering::equal;
+    }
+    // One or both are null
+    if (this->t == Type::Null) {
+        if (other.t == Type::PreRelease) {
+            return std::strong_ordering::greater;
+        }
+        return std::strong_ordering::less;
+    }
+    if (other.t == Type::Null) {
+        if (this->t == Type::PreRelease) {
+            return std::strong_ordering::less;
+        }
+        return std::strong_ordering::greater;
+    }
+    // Textual comparison (differing type, or both textual/pre-release)
+    auto minLen = qMin(this->value.size(), other.value.size());
+    for (int i = 0; i < minLen; i++) {
+        auto a = this->value.at(i);
+        auto b = other.value.at(i);
+        if (a != b) {
+            // Compare by rune
+            return a.unicode() <=> b.unicode();
+        }
+    }
+    // Compare by length
+    return this->value.size() <=> other.value.size();
+}
+
+namespace {
+void removeLeadingZeros(QString& s)
+{
+    s.remove(0, std::distance(s.begin(), std::ranges::find_if_not(s, [](QChar c) { return c == '0'; })));
+}
+}  // namespace
+
+void Version::parse()
+{
+    auto len = m_string.size();
+    for (int i = 0; i < len;) {
+        Section cur(Section::Type::Textual);
+        auto c = m_string.at(i);
+        if (c == '+') {
+            break;  // Ignore appendices
+        }
+        if (c == '-') {
+            // Add dash to component
+            cur.value += '-';
+            i++;
+            // If the next rune is non-digit, mark as pre-release (requires >= 1 non-digit after dash so the component has length > 1)
+            if (i < len && !m_string.at(i).isDigit()) {
+                cur.t = Section::Type::PreRelease;
+            }
+        } else if (c.isDigit()) {
+            // Mark as numeric
+            cur.t = Section::Type::Numeric;
+        }
+        for (; i < len; i++) {
+            auto r = m_string.at(i);
+            if ((r.isDigit() != (cur.t == Section::Type::Numeric)) ||
+                (r == '-' && cur.t != Section::Type::PreRelease)  // "---" is a valid pre-release component
+                || r == '+') {
+                // Run completed (do not consume this rune)
+                break;
+            }
+            // Add rune to current run
+            cur.value += r;
+        }
+        if (!cur.value.isEmpty()) {
+            if (cur.t == Section::Type::Numeric) {
+                removeLeadingZeros(cur.value);
+            }
+            m_sections.append(cur);
+        }
+    }
+}
+
+std::strong_ordering Version::operator<=>(const Version& other) const
+{
+    const auto size = qMax(m_sections.size(), other.m_sections.size());
+    for (int i = 0; i < size; ++i) {
+        auto sec1 = (i >= m_sections.size()) ? Section() : m_sections.at(i);
+        auto sec2 = (i >= other.m_sections.size()) ? Section() : other.m_sections.at(i);
+
+        if (auto cmp = sec1 <=> sec2; cmp != std::strong_ordering::equal) {
+            return cmp;
+        }
+    }
+    return std::strong_ordering::equal;
+}

launcher/Version.h

@@ -3,6 +3,7 @@
  *  Prism Launcher - Minecraft Launcher
  *  Copyright (C) 2023 flowln <flowlnlnln@gmail.com>
  *  Copyright (C) 2022 Sefa Eyeoglu <contact@scrumplex.net>
+ *  Copyright (c) 2026 Trial97 <alexandru.tripon97@gmail.com>
  *
  *  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
@@ -15,23 +16,6 @@
  *
  *  You should have received a copy of the GNU General Public License
  *  along with this program.  If not, see <https://www.gnu.org/licenses/>.
- *
- * This file incorporates work covered by the following copyright and
- * permission notice:
- *
- *      Copyright 2013-2021 MultiMC Contributors
- *
- *      Licensed under the Apache License, Version 2.0 (the "License");
- *      you may not use this file except in compliance with the License.
- *      You may obtain a copy of the License at
- *
- *          http://www.apache.org/licenses/LICENSE-2.0
- *
- *      Unless required by applicable law or agreed to in writing, software
- *      distributed under the License is distributed on an "AS IS" BASIS,
- *      WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- *      See the License for the specific language governing permissions and
- *      limitations under the License.
  */
 
 #pragma once
@@ -41,115 +25,36 @@
 #include <QString>
 #include <QStringView>
 
-class QUrl;
-
+// this implements the FlexVer
+// https://git.sleeping.town/exa/FlexVer
 class Version {
    public:
-    Version(QString str);
+    Version(QString str) : m_string(std::move(str)) { parse(); }
     Version() = default;
 
-    bool operator<(const Version& other) const;
-    bool operator<=(const Version& other) const;
-    bool operator>(const Version& other) const;
-    bool operator>=(const Version& other) const;
-    bool operator==(const Version& other) const;
-    bool operator!=(const Version& other) const;
+   private:
+    struct Section {
+        enum class Type : std::uint8_t { Null, Textual, Numeric, PreRelease };
+        explicit Section(Type t = Type::Null, QString value = "") : t(t), value(std::move(value)) {}
+        Type t;
+        QString value;
+        bool operator==(const Section& other) const = default;
+        std::strong_ordering operator<=>(const Section& other) const;
+    };
 
+   private:
+    void parse();
+
+   public:
     QString toString() const { return m_string; }
     bool isEmpty() const { return m_string.isEmpty(); }
 
     friend QDebug operator<<(QDebug debug, const Version& v);
 
-   private:
-    struct Section {
-        explicit Section(QString fullString) : m_fullString(std::move(fullString))
-        {
-            qsizetype cutoff = m_fullString.size();
-            for (int i = 0; i < m_fullString.size(); i++) {
-                if (!m_fullString[i].isDigit()) {
-                    cutoff = i;
-                    break;
-                }
-            }
-
-            auto numPart = QStringView{ m_fullString }.left(cutoff);
-
-            if (!numPart.isEmpty()) {
-                m_isNull = false;
-                m_numPart = numPart.toInt();
-            }
-
-            auto stringPart = QStringView{ m_fullString }.mid(cutoff);
-
-            if (!stringPart.isEmpty()) {
-                m_isNull = false;
-                m_stringPart = stringPart.toString();
-            }
-        }
-
-        explicit Section() = default;
-
-        bool m_isNull = true;
-
-        int m_numPart = 0;
-        QString m_stringPart;
-
-        QString m_fullString;
-
-        inline bool isAppendix() const { return m_stringPart.startsWith('+'); }
-        inline bool isPreRelease() const { return m_stringPart.startsWith('-') && m_stringPart.length() > 1; }
-
-        inline bool operator==(const Section& other) const
-        {
-            if (m_isNull && !other.m_isNull)
-                return false;
-            if (!m_isNull && other.m_isNull)
-                return false;
-
-            if (!m_isNull && !other.m_isNull) {
-                return (m_numPart == other.m_numPart) && (m_stringPart == other.m_stringPart);
-            }
-
-            return true;
-        }
-
-        inline bool operator<(const Section& other) const
-        {
-            static auto unequal_is_less = [](const Section& non_null) -> bool {
-                if (non_null.m_stringPart.isEmpty())
-                    return non_null.m_numPart == 0;
-                return (non_null.m_stringPart != QLatin1Char('.')) && non_null.isPreRelease();
-            };
-
-            if (!m_isNull && other.m_isNull)
-                return unequal_is_less(*this);
-            if (m_isNull && !other.m_isNull)
-                return !unequal_is_less(other);
-
-            if (!m_isNull && !other.m_isNull) {
-                if (m_numPart < other.m_numPart)
-                    return true;
-                if (m_numPart == other.m_numPart && m_stringPart < other.m_stringPart)
-                    return true;
-
-                if (!m_stringPart.isEmpty() && other.m_stringPart.isEmpty())
-                    return false;
-                if (m_stringPart.isEmpty() && !other.m_stringPart.isEmpty())
-                    return true;
-
-                return false;
-            }
-
-            return m_fullString < other.m_fullString;
-        }
-
-        inline bool operator!=(const Section& other) const { return !(*this == other); }
-        inline bool operator>(const Section& other) const { return !(*this < other || *this == other); }
-    };
+    bool operator==(const Version& other) const { return (*this <=> other) == std::strong_ordering::equal; }
+    std::strong_ordering operator<=>(const Version& other) const;
 
    private:
     QString m_string;
     QList<Section> m_sections;
-
-    void parse();
-};
+};

launcher/modplatform/packwiz/Packwiz.cpp

@@ -22,6 +22,8 @@
 #include <QDebug>
 #include <QDir>
 #include <QObject>
+#include <algorithm>
+#include <compare>
 #include <sstream>
 #include <string>
 #include <utility>
@@ -91,6 +93,15 @@ auto intEntry(toml::table table, QString entry_name) -> int
     return node.value_or(0);
 }
 
+bool sortMCVersions(const QString& a, const QString& b)
+{
+    auto cmp = Version(a) <=> Version(b);
+    if (cmp == std::strong_ordering::equal) {
+        return a < b;
+    }
+    return cmp == std::strong_ordering::less;
+};
+
 auto V1::createModFormat([[maybe_unused]] const QDir& index_dir,
                          ModPlatform::IndexedPack& mod_pack,
                          ModPlatform::IndexedVersion& mod_version) -> Mod
@@ -117,8 +128,8 @@ auto V1::createModFormat([[maybe_unused]] const QDir& index_dir,
     mod.side = mod_version.side == ModPlatform::Side::NoSide ? mod_pack.side : mod_version.side;
     mod.loaders = mod_version.loaders;
     mod.mcVersions = mod_version.mcVersion;
-    std::sort(mod.mcVersions.begin(), mod.mcVersions.end(),
-              [](QString a, QString b) { return Version(std::move(a)) < Version(std::move(b)); });
+    mod.mcVersions.removeDuplicates();
+    std::ranges::sort(mod.mcVersions, sortMCVersions);
     mod.releaseType = mod_version.version_type;
 
     mod.version_number = mod_version.version_number;
@@ -304,8 +315,8 @@ auto V1::getIndexForMod(const QDir& index_dir, QString slug) -> Mod
                     }
                 }
             }
-            std::sort(mod.mcVersions.begin(), mod.mcVersions.end(),
-                      [](QString a, QString b) { return Version(std::move(a)) < Version(std::move(b)); });
+            mod.mcVersions.removeDuplicates();
+            std::ranges::sort(mod.mcVersions, sortMCVersions);
         }
     }
     mod.version_number = table["x-prismlauncher-version-number"].value_or("");