CVE-2022-25297 (C++, published 2022-02-21T08:15Z)
Description: value: This affects the package drogonframework/drogon before 1.7.5. The unsafe handling of file names during upload using HttpFile::save() method may enable attackers to write files to arbitrary locations outside the designated target folder.
Severity: MEDIUM (CVSS v3 base score: 8.8)
Weakness: CWE-552 \u2014 Files or Directories Accessible to External Parties
The product makes files or directories accessible to unauthorized actors, even though they should not be.
Repository: https://github.com/drogonframework/drogon
Files changed:
lib/src/HttpFileImpl.cc
lib/tests/CMakeLists.txt
lib/tests/unittests/HttpFileTest.cc
Commit message: Prevent malformed upload path causing arbitrary write (#1174)



Vulnerable code:
    filesystem::path fsPath(utils::toNativePath(path));
    if (!fsPath.is_absolute() &&
        (!fsPath.has_parent_path() ||
         (fsPath.begin()->string() != "." && fsPath.begin()->string() != "..")))
        filesystem::path fsUploadPath(utils::toNativePath(
            HttpAppFrameworkImpl::instance().getUploadPath()));
        fsPath = fsUploadPath / fsPath;
    filesystem::path fsFileName(utils::toNativePath(fileName_));
    if (!filesystem::exists(fsPath))
        LOG_TRACE << "create path:" << fsPath;
        filesystem::create_directories(fsPath, err);
    return saveTo(fsPath / fsFileName);
set(UNITTEST_SOURCES unittests/main.cc
                        unittests/Base64Test.cc
                        unittests/UrlCodecTest.cc
                        unittests/GzipTest.cc
                        unittests/HttpViewDataTest.cc
                        unittests/CookieTest.cc
                        unittests/ClassNameTest.cc
                        unittests/HttpDateTest.cc
                        unittests/HttpHeaderTest.cc
                        unittests/MD5Test.cc
                        unittests/MsgBufferTest.cc
                        unittests/OStringStreamTest.cc
                        unittests/PubSubServiceUnittest.cc
                        unittests/Sha1Test.cc
                        ../src/ssl_funcs/Sha1.cc
                        ../src/HttpUtils.cc
                        unittests/FileTypeTest.cc
                        unittests/DrObjectTest.cc
                        unittests/HttpFullDateTest.cc
                        unittests/MainLoopTest.cc
                        unittests/CacheMapTest.cc
                        unittests/StringOpsTest.cc)
    set(UNITTEST_SOURCES ${UNITTEST_SOURCES} unittests/CoroutineTest.cc)
    set(UNITTEST_SOURCES ${UNITTEST_SOURCES} unittests/BrotliTest.cc)
if (CMAKE_CXX_COMPILER_ID MATCHES "MSVC" AND BUILD_DROGON_SHARED)
    set(UNITTEST_SOURCES ${UNITTEST_SOURCES} ../src/HttpUtils.cc)
set(INTEGRATION_TEST_CLIENT_SOURCES integration_test/client/main.cc
                                    integration_test/client/WebSocketTest.cc
                                    integration_test/client/MultipleWsTest.cc
                                    integration_test/client/HttpPipeliningTest.cc)
    set(INTEGRATION_TEST_SERVER_SOURCES ${INTEGRATION_TEST_SERVER_SOURCES}
        integration_test/server/api_v1_CoroTest.cc)
    set(CMAKE_CXX_STANDARD 20)
    set(CMAKE_CXX_STANDARD_REQUIRED TRUE)
set_property(TARGET ${tests}
             PROPERTY CXX_STANDARD ${DROGON_CXX_STANDARD})
ParseAndAddDrogonTests(unittest)

Fixed code:
#include <algorithm>
    filesystem::path fsUploadDir(utils::toNativePath(path));

    if (!fsUploadDir.is_absolute() && (!fsUploadDir.has_parent_path() ||
                                       (fsUploadDir.begin()->string() != "." &&
                                        fsUploadDir.begin()->string() != "..")))
        fsUploadDir = utils::toNativePath(
                          HttpAppFrameworkImpl::instance().getUploadPath()) /
                      fsUploadDir;

    fsUploadDir = filesystem::weakly_canonical(fsUploadDir);

    if (!filesystem::exists(fsUploadDir))
        LOG_TRACE << "create path:" << fsUploadDir;
        filesystem::create_directories(fsUploadDir, err);

    filesystem::path fsSaveToPath(filesystem::weakly_canonical(
        fsUploadDir / utils::toNativePath(fileName_)));

    if (!std::equal(fsUploadDir.begin(),
                    fsUploadDir.end(),
                    fsSaveToPath.begin()))
    {
        LOG_ERROR
            << "Attempt writing outside of upload directory detected. Path: "
            << fileName_;
        return -1;
    }

    return saveTo(fsSaveToPath);
set(UNITTEST_SOURCES
    unittests/main.cc
    unittests/Base64Test.cc
    unittests/UrlCodecTest.cc
    unittests/GzipTest.cc
    unittests/HttpViewDataTest.cc
    unittests/CookieTest.cc
    unittests/ClassNameTest.cc
    unittests/HttpDateTest.cc
    unittests/HttpHeaderTest.cc
    unittests/MD5Test.cc
    unittests/MsgBufferTest.cc
    unittests/OStringStreamTest.cc
    unittests/PubSubServiceUnittest.cc
    unittests/Sha1Test.cc
    ../src/ssl_funcs/Sha1.cc
    unittests/FileTypeTest.cc
    unittests/DrObjectTest.cc
    unittests/HttpFullDateTest.cc
    unittests/MainLoopTest.cc
    unittests/CacheMapTest.cc
    unittests/StringOpsTest.cc)
  set(UNITTEST_SOURCES ${UNITTEST_SOURCES} unittests/CoroutineTest.cc)
  set(UNITTEST_SOURCES ${UNITTEST_SOURCES} unittests/BrotliTest.cc)
if(CMAKE_CXX_COMPILER_ID MATCHES "MSVC" AND BUILD_DROGON_SHARED)
  set(UNITTEST_SOURCES ${UNITTEST_SOURCES} ../src/HttpUtils.cc)
else()
  set(UNITTEST_SOURCES ${UNITTEST_SOURCES} ../src/HttpFileImpl.cc
                       unittests/HttpFileTest.cc)
set(INTEGRATION_TEST_CLIENT_SOURCES
    integration_test/client/main.cc
    integration_test/client/WebSocketTest.cc
    integration_test/client/MultipleWsTest.cc
    integration_test/client/HttpPipeliningTest.cc)
  set(INTEGRATION_TEST_SERVER_SOURCES
      ${INTEGRATION_TEST_SERVER_SOURCES}
      integration_test/server/api_v1_CoroTest.cc)
  set(CMAKE_CXX_STANDARD 20)
  set(CMAKE_CXX_STANDARD_REQUIRED TRUE)
set_property(TARGET ${tests} PROPERTY CXX_STANDARD ${DROGON_CXX_STANDARD})
parseandadddrogontests(unittest)
#include "../../lib/src/HttpFileImpl.h"
#include <drogon/drogon_test.h>
#include <filesystem>

using namespace drogon;
using namespace std;

DROGON_TEST(HttpFile)
{
    SUBSECTION(Save)
    {
        HttpFileImpl file;
        file.setFileName("test_file_name");
        file.setFile("test", 4);
        auto out = file.save("./test_uploads_dir");

        CHECK(out == 0);
        CHECK(filesystem::exists("./test_uploads_dir/test_file_name"));

        filesystem::remove_all("./test_uploads_dir");
    }

    SUBSECTION(SavePathRelativeTraversal)
    {
        auto uploadPath = filesystem::current_path() / "test_uploads_dir";

        HttpFileImpl file;
        file.setFileName("../test_malicious_file_name");
        file.setFile("test", 4);
        auto out = file.save(uploadPath.string());

        CHECK(out == -1);
        CHECK(!filesystem::exists(uploadPath / "../test_malicious_file_name"));

        filesystem::remove_all(uploadPath);
        filesystem::remove(uploadPath / "../test_malicious_file_name");
    }

    SUBSECTION(SavePathAbsoluteTraversal)
    {
        HttpFileImpl file;
        file.setFileName("/tmp/test_malicious_file_name");
        file.setFile("test", 4);
        auto out = file.save("./test_uploads_dir");

        CHECK(out == -1);
        CHECK(!filesystem::exists("/tmp/test_malicious_file_name"));

        filesystem::remove_all("test_uploads_dir");
        filesystem::remove_all("/tmp/test_malicious_file_name");
    }
}