pencil2d · scribblemaniac · Jun 6, 2024 · May 23, 2024 · May 24, 2024 · May 24, 2024
diff --git a/core_lib/src/structure/filemanager.cpp b/core_lib/src/structure/filemanager.cpp
@@ -24,6 +24,7 @@ GNU General Public License for more details.
 #include "fileformat.h"
 #include "object.h"
 #include "layercamera.h"
+#include "util/util.h"
 
 FileManager::FileManager(QObject* parent) : QObject(parent)
 {
@@ -612,8 +613,8 @@ bool FileManager::loadPalette(Object* obj)
 {
     FILEMANAGER_LOG("Load Palette..");
 
-    QString paletteFilePath = QDir(obj->dataDir()).filePath(PFF_PALETTE_FILE);
-    if (!obj->importPalette(paletteFilePath))
+    QString paletteFilePath = validateDataPath(PFF_PALETTE_FILE, obj->dataDir());
+    if (paletteFilePath.isEmpty() || !obj->importPalette(paletteFilePath))
     {
         obj->loadDefaultPalette();
     }

diff --git a/core_lib/src/structure/layerbitmap.cpp b/core_lib/src/structure/layerbitmap.cpp
@@ -21,7 +21,7 @@ GNU General Public License for more details.
 #include <QFile>
 #include "keyframe.h"
 #include "bitmapimage.h"
-
+#include "util/util.h"
 
 LayerBitmap::LayerBitmap(int id) : Layer(id, Layer::BITMAP)
 {
@@ -202,24 +202,19 @@ void LayerBitmap::loadDomElement(const QDomElement& element, QString dataDirPath
     while (!imageTag.isNull())
     {
         QDomElement imageElement = imageTag.toElement();
-        if (!imageElement.isNull())
+        if (!imageElement.isNull() && imageElement.tagName() == "image")
         {
-            if (imageElement.tagName() == "image")
+            QString path = validateDataPath(imageElement.attribute("src"), dataDirPath);
+            if (!path.isEmpty())
             {
-                QString path = dataDirPath + "/" + imageElement.attribute("src"); // the file is supposed to be in the data directory
-                QFileInfo fi(path);
-                if (!fi.exists()) path = imageElement.attribute("src");
                 int position = imageElement.attribute("frame").toInt();
                 int x = imageElement.attribute("topLeftX").toInt();
                 int y = imageElement.attribute("topLeftY").toInt();
-                qreal opacity = 1.0;
-                if (imageElement.hasAttribute("opacity")) {
-                    opacity = imageElement.attribute("opacity").toDouble();
-                }
+                qreal opacity = imageElement.attribute("opacity", "1.0").toDouble();
                 loadImageAtFrame(path, QPoint(x, y), position, opacity);
-
-                progressStep();
             }
+
+            progressStep();
         }
         imageTag = imageTag.nextSibling();
     }

diff --git a/core_lib/src/structure/layersound.cpp b/core_lib/src/structure/layersound.cpp
@@ -21,6 +21,7 @@ GNU General Public License for more details.
 #include <QFileInfo>
 #include <QDir>
 #include "soundclip.h"
+#include "util/util.h"
 
 
 LayerSound::LayerSound(int id) : Layer(id, Layer::SOUND)
@@ -93,24 +94,21 @@ void LayerSound::loadDomElement(const QDomElement& element, QString dataDirPath,
     while (!soundTag.isNull())
     {
         QDomElement soundElement = soundTag.toElement();
-        if (soundElement.isNull())
-        {
-            continue;
-        }
 
-        if (soundElement.tagName() == "sound")
+        if (!soundElement.isNull() && soundElement.tagName() == "sound")
         {
             const QString soundFile = soundElement.attribute("src");
             const QString sSoundClipName = soundElement.attribute("name", "My Sound Clip");
 
             if (!soundFile.isEmpty())
             {
-                // the file is supposed to be in the data directory
-                const QString sFullPath = QDir(dataDirPath).filePath(soundFile);
-
-                int position = soundElement.attribute("frame").toInt();
-                Status st = loadSoundClipAtFrame(sSoundClipName, sFullPath, position);
-                Q_ASSERT(st.ok());
+                QString path = validateDataPath(soundFile, dataDirPath);
+                if (!path.isEmpty())
+                {
+                    int position = soundElement.attribute("frame").toInt();
+                    Status st = loadSoundClipAtFrame(sSoundClipName, path, position);
+                    Q_ASSERT(st.ok());
+                }
             }
             progressStep();
         }

diff --git a/core_lib/src/structure/layervector.cpp b/core_lib/src/structure/layervector.cpp
@@ -20,7 +20,7 @@ GNU General Public License for more details.
 #include <QDir>
 #include <QFile>
 #include <QFileInfo>
-
+#include "util/util.h"
 
 LayerVector::LayerVector(int id) : Layer(id, Layer::VECTOR)
 {
@@ -155,31 +155,31 @@ void LayerVector::loadDomElement(const QDomElement& element, QString dataDirPath
     while (!imageTag.isNull())
     {
         QDomElement imageElement = imageTag.toElement();
-        if (!imageElement.isNull())
+        if (!imageElement.isNull() && imageElement.tagName() == "image")
         {
-            if (imageElement.tagName() == "image")
+            int position;
+            QString rawPath = imageElement.attribute("src");
+            if (!rawPath.isNull())
             {
-                int position;
-                if (!imageElement.attribute("src").isNull())
+                QString path = validateDataPath(rawPath, dataDirPath);
+                if (!path.isEmpty())
                 {
-                    QString path = dataDirPath + "/" + imageElement.attribute("src"); // the file is supposed to be in the data directory
-                    QFileInfo fi(path);
-                    if (!fi.exists()) path = imageElement.attribute("src");
                     position = imageElement.attribute("frame").toInt();
                     loadImageAtFrame(path, position);
+                    getVectorImageAtFrame(position)->setOpacity(imageElement.attribute("opacity", "1.0").toDouble());
                 }
-                else
-                {
-                    position = imageElement.attribute("frame").toInt();
-                    addNewKeyFrameAt(position);
-                    getVectorImageAtFrame(position)->loadDomElement(imageElement);
-                }
-                if (imageElement.hasAttribute("opacity"))
-                    getVectorImageAtFrame(position)->setOpacity(imageElement.attribute("opacity").toDouble());
-                else
-                    getVectorImageAtFrame(position)->setOpacity(1.0);
-                progressStep();
             }
+            else
+            {
+                position = imageElement.attribute("frame").toInt();
+                addNewKeyFrameAt(position);
+                getVectorImageAtFrame(position)->loadDomElement(imageElement);
+                getVectorImageAtFrame(position)->setOpacity(imageElement.attribute("opacity", "1.0").toDouble());
+            }
+
+
+
+            progressStep();
         }
         imageTag = imageTag.nextSibling();
     }

diff --git a/core_lib/src/util/util.cpp b/core_lib/src/util/util.cpp
@@ -17,6 +17,8 @@ GNU General Public License for more details.
 #include "util.h"
 #include <QAbstractSpinBox>
 #include <QApplication>
+#include <QDir>
+#include <QFileInfo>
 #include <QStandardPaths>
 
 static inline bool clipLineToEdge(qreal& t0, qreal& t1, qreal p, qreal q)
@@ -126,3 +128,59 @@ QString uniqueString(int len)
     s[len] = 0;
     return QString::fromUtf8(s);
 }
+
+QString validateDataPath(QString filePath, QString dataDirPath)
+{
+    // Make sure src path is relative
+    if (!QFileInfo(filePath).isRelative()) return QString();
+
+    QFileInfo fi(dataDirPath, filePath);
+    // Recursively resolve symlinks
+    QString canonicalPath = fi.canonicalFilePath();
+
+    QDir dataDir(dataDirPath);
+    // Resolve symlinks in data dir path so it can be compared against file paths with resolved symlinks
+    if (dataDir.exists())
+    {
+        dataDir.setPath(dataDir.canonicalPath());
+    }
+    // Iterate over parent directories of the file path to see if one of them equals the data directory
+    if (canonicalPath.isEmpty())
+    {
+        // File does not exist, use absolute path and attempt to resolve symlinks again for each parent directory
+        fi.setFile(fi.absoluteFilePath());
+        QDir ancestor(fi.absoluteFilePath());
+        while (ancestor != dataDir) {
+            if (ancestor.isRoot())
+            {
+                // Reached root directory without finding data dir
+                return QString();
+            }
+            QDir newAncestor = QFileInfo(ancestor.absolutePath()).dir();
+            if (newAncestor.exists())
+            {
+                // Resolve directory symlinks
+                newAncestor.setPath(newAncestor.canonicalPath());
+            }
+            ancestor = newAncestor;
+        }
+        // One of the parent directories of filePath matches dataDir
+        return fi.absoluteFilePath();
+    }
+    else
+    {
+        // File exists and all symlinks have been resolved in canonicalPath so no further attempts to resolve symlinks are necessary
+        fi.setFile(canonicalPath);
+        QDir ancestor = fi.dir();
+        while (ancestor != dataDir)
+        {
+            if (ancestor.isRoot()) {
+                // Data dir was not found in ancestors of the src path
+                return QString();
+            }
+            ancestor = QFileInfo(ancestor.absolutePath()).dir();
+        }
+        // One of the parent directories of filePath matches dataDir
+        return fi.absoluteFilePath();
+    }
+}
diff --git a/core_lib/src/util/util.h b/core_lib/src/util/util.h
@@ -68,4 +68,26 @@ QString ffmpegLocation();
 quint64 imageSize(const QImage&);
 QString uniqueString(int len);
 
+/**
+ * Performs safety checks for paths to data directory assets.
+ *
+ * Validates that the given file path is contained within the given
+ * data directory after resolving symlinks. Also requires paths to
+ * be relative to prevent project portability issues or intentional
+ * platform-dependent behavior.
+ *
+ * This function does not verify if the path actually exists.
+ *
+ * This function should be called for every file being read from the data directory.
+ * For writing files to the data directory, it is only necessary to call this
+ * function if:
+ * - An existing file is being modified/appended in-place (not overwritten) in the data directory.
+ * - The data directory is not guaranteed to be the immediate parent directory of the file being written.
+ *
+ * @param filePath A path to a data file.
+ * @param dataDir The path to the data directory.
+ * @return The valid resolved path, or empty if the path is not valid.
+ */
+QString validateDataPath(QString filePath, QString dataDirPath);
+
 #endif // UTIL_H
diff --git a/tests/src/test_layerbitmap.cpp b/tests/src/test_layerbitmap.cpp
@@ -0,0 +1,117 @@
+/*
+
+Pencil2D - Traditional Animation Software
+Copyright (C) 2012-2020 Matthew Chiawen Chang
+
+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 2 of the License.
+
+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.
+
+*/
+#include "catch.hpp"
+
+#include "layerbitmap.h"
+#include "bitmapimage.h"
+
+#include <memory>
+#include <QDir>
+#include <QDomElement>
+#include <QTemporaryDir>
+
+TEST_CASE("Load bitmap layer from XML")
+{
+    std::unique_ptr<Layer> bitmapLayer(new LayerBitmap(1));
+    QTemporaryDir dataDir;
+    REQUIRE(dataDir.isValid());
+    QDomDocument doc;
+    doc.setContent(QString("<layer id='1' name='Bitmap Layer' visibility='1'></layer>"));
+    QDomElement layerElem = doc.documentElement();
+    ProgressCallback nullCallback = []() {};
+
+    auto createFrame = [&layerElem, &doc](QString src = "001.001.png", int frame = 1, int topLeftX = 0, int topLeftY = 0)
+    {
+        QDomElement frameElem = doc.createElement("image");
+        frameElem.setAttribute("src", src);
+        frameElem.setAttribute("frame", frame);
+        frameElem.setAttribute("topLeftX", topLeftX);
+        frameElem.setAttribute("topLeftY", topLeftY);
+        layerElem.appendChild(frameElem);
+    };
+
+    SECTION("No frames")
+    {
+        bitmapLayer->loadDomElement(layerElem, dataDir.path(), []() {});
+
+        REQUIRE(bitmapLayer->keyFrameCount() == 0);
+    }
+
+    SECTION("Single frame")
+    {
+        createFrame("001.001.png", 1, 0, 0);
+
+        bitmapLayer->loadDomElement(layerElem, dataDir.path(), nullCallback);
+
+        REQUIRE(bitmapLayer->keyFrameCount() == 1);
+        BitmapImage* frame = static_cast<BitmapImage*>(bitmapLayer->getKeyFrameAt(1));
+        REQUIRE(frame != nullptr);
+        REQUIRE(frame->top() == 0);
+        REQUIRE(frame->left() == 0);
+        REQUIRE(QDir(frame->fileName()) == QDir(dataDir.filePath("001.001.png")));
+    }
+
+    SECTION("Multiple frames")
+    {
+        createFrame("001.001.png", 1);
+        createFrame("001.002.png", 2);
+
+        bitmapLayer->loadDomElement(layerElem, dataDir.path(), nullCallback);
+
+        REQUIRE(bitmapLayer->keyFrameCount() == 2);
+        for (int i = 1; i <= 2; i++)
+        {
+            BitmapImage* frame = static_cast<BitmapImage*>(bitmapLayer->getKeyFrameAt(i));
+            REQUIRE(frame != nullptr);
+            REQUIRE(frame->top() == 0);
+            REQUIRE(frame->left() == 0);
+            REQUIRE(QDir(frame->fileName()) == QDir(dataDir.filePath(QString("001.%1.png").arg(QString::number(i), 3, QChar('0')))));
+        }
+    }
+
+    SECTION("Frame with absolute src")
+    {
+        createFrame(QDir(dataDir.filePath("001.001.png")).absolutePath());
+
+        bitmapLayer->loadDomElement(layerElem, dataDir.path(), nullCallback);
+
+        REQUIRE(bitmapLayer->keyFrameCount() == 0);
+    }
+
+    SECTION("Frame src outside of data dir")
+    {
+        QTemporaryDir otherDir;
+        createFrame(QDir(dataDir.path()).relativeFilePath(QDir(otherDir.filePath("001.001.png")).absolutePath()));
+
+        bitmapLayer->loadDomElement(layerElem, dataDir.path(), nullCallback);
+
+        REQUIRE(bitmapLayer->keyFrameCount() == 0);
+    }
+
+    SECTION("Frame src nested in data dir")
+    {
+        createFrame("subdir/001.001.png");
+
+        bitmapLayer->loadDomElement(layerElem, dataDir.path(), nullCallback);
+
+        REQUIRE(bitmapLayer->keyFrameCount() == 1);
+        BitmapImage* frame = static_cast<BitmapImage*>(bitmapLayer->getKeyFrameAt(1));
+        REQUIRE(frame != nullptr);
+        REQUIRE(frame->top() == 0);
+        REQUIRE(frame->left() == 0);
+        REQUIRE(QDir(frame->fileName()) == QDir(dataDir.filePath("subdir/001.001.png")));
+    }
+}