From 6117e1aa2b1abc38d750e25df816e88e19fb06be Mon Sep 17 00:00:00 2001 From: Florian Reimold <11774314+FlorianReimold@users.noreply.github.com> Date: Wed, 25 Oct 2023 11:08:53 +0200 Subject: [PATCH] Added Support for APPE again (and fixed the Win32 code to support it) --- fineftp-server/src/ftp_session.cpp | 9 ++ fineftp-server/src/win32/file_man.cpp | 22 +++- tests/fineftp_test/src/fineftp_test.cpp | 159 ++++++++++++++++++++++++ 3 files changed, 187 insertions(+), 3 deletions(-) diff --git a/fineftp-server/src/ftp_session.cpp b/fineftp-server/src/ftp_session.cpp index 37a97e9..32bcc8c 100755 --- a/fineftp-server/src/ftp_session.cpp +++ b/fineftp-server/src/ftp_session.cpp @@ -621,7 +621,12 @@ namespace fineftp if (!file->good()) { +#ifdef WIN32 + sendFtpMessage(FtpReplyCode::ACTION_ABORTED_LOCAL_ERROR, "Error opening file for transfer: " + GetLastErrorStr()); +#else sendFtpMessage(FtpReplyCode::ACTION_ABORTED_LOCAL_ERROR, "Error opening file for transfer"); +#endif // WIN32 + return; } @@ -667,7 +672,11 @@ namespace fineftp if (!file->good()) { +#ifdef WIN32 + sendFtpMessage(FtpReplyCode::ACTION_ABORTED_LOCAL_ERROR, "Error opening file for transfer: " + GetLastErrorStr()); +#else sendFtpMessage(FtpReplyCode::ACTION_ABORTED_LOCAL_ERROR, "Error opening file for transfer"); +#endif // WIN32 return; } diff --git a/fineftp-server/src/win32/file_man.cpp b/fineftp-server/src/win32/file_man.cpp index 3b1bd4a..b6547f4 100644 --- a/fineftp-server/src/win32/file_man.cpp +++ b/fineftp-server/src/win32/file_man.cpp @@ -112,12 +112,28 @@ WriteableFile::WriteableFile(const std::string& filename, std::ios::openmode mod { // std::ios::binary is ignored in mode because, on Windows, even ASCII files have to be stored as // binary files as they come in with the right line endings. - (void)mode; + + DWORD dwDesiredAccess = GENERIC_WRITE; // It's always a writeable file + if (mode & std::ios::app) + { + dwDesiredAccess |= FILE_APPEND_DATA; // Append to the file + } + + DWORD dwCreationDisposition = 0; + if (mode & std::ios::app) + { + dwCreationDisposition = OPEN_EXISTING; // Append => Open existing file + } + else + { + dwCreationDisposition = CREATE_ALWAYS; // Not Append => Create new file + } + #if !defined(__GNUG__) auto wfilename = StrConvert::Utf8ToWide(filename); - handle_ = ::CreateFileW(wfilename.c_str(), GENERIC_WRITE, FILE_SHARE_DELETE, nullptr, CREATE_NEW, FILE_ATTRIBUTE_NORMAL /*FILE_FLAG_WRITE_THROUGH*/, 0); + handle_ = ::CreateFileW(wfilename.c_str(), dwDesiredAccess, FILE_SHARE_DELETE, nullptr, dwCreationDisposition, FILE_ATTRIBUTE_NORMAL, 0); #else - handle_ = ::CreateFileA(filename.c_str(), GENERIC_WRITE, FILE_SHARE_DELETE, nullptr, CREATE_NEW, /*FILE_ATTRIBUTE_NORMAL*/ FILE_FLAG_WRITE_THROUGH, 0); + handle_ = ::CreateFileA(filename.c_str(), dwDesiredAccess, FILE_SHARE_DELETE, nullptr, dwCreationDisposition, FILE_ATTRIBUTE_NORMAL, 0); #endif if (INVALID_HANDLE_VALUE != handle_ && (mode & std::ios::app) == std::ios::app) diff --git a/tests/fineftp_test/src/fineftp_test.cpp b/tests/fineftp_test/src/fineftp_test.cpp index 16268d5..e66dce4 100644 --- a/tests/fineftp_test/src/fineftp_test.cpp +++ b/tests/fineftp_test/src/fineftp_test.cpp @@ -904,6 +904,165 @@ TEST(FineFTPTest, UTF8Paths) } #endif +#if 1 +TEST(FineFTPTest, AppendToFile) { + auto test_working_dir = std::filesystem::current_path(); + auto ftp_root_dir = test_working_dir / "ftp_root"; + auto local_root_dir = test_working_dir / "local_root"; + + { + if (std::filesystem::exists(ftp_root_dir)) + std::filesystem::remove_all(ftp_root_dir); + + if (std::filesystem::exists(local_root_dir)) + std::filesystem::remove_all(local_root_dir); + + // Make sure that we start clean, so no old dir exists + ASSERT_FALSE(std::filesystem::exists(ftp_root_dir)); + ASSERT_FALSE(std::filesystem::exists(local_root_dir)); + + std::filesystem::create_directory(ftp_root_dir); + std::filesystem::create_directory(local_root_dir); + + // Make sure that we were able to create the dir + ASSERT_TRUE(std::filesystem::is_directory(ftp_root_dir)); + ASSERT_TRUE(std::filesystem::is_directory(local_root_dir)); + } + + fineftp::FtpServer server(2121); + server.start(1); + + server.addUserAnonymous(ftp_root_dir.string(), fineftp::Permission::All); + + // Create a hello world.txt file in the local root dir and write "Hello World" into it + auto local_file = local_root_dir / "hello_world.txt"; + { + std::ofstream ofs(local_file.string()); + ofs << "Hello World"; + ofs.close(); + } + + // Make sure that the file exists + ASSERT_TRUE(std::filesystem::exists(local_file)); + ASSERT_TRUE(std::filesystem::is_regular_file(local_file)); + + // create a hello world.txt file in the ftp root dir and write "HELLO WORLD" into it + auto ftp_file = ftp_root_dir / "hello_world.txt"; + { + std::ofstream ofs(ftp_file.string()); + ofs << "HELLO WORLD"; + ofs.close(); + } + + // Make sure that the file exists + ASSERT_TRUE(std::filesystem::exists(ftp_file)); + ASSERT_TRUE(std::filesystem::is_regular_file(ftp_file)); + + // Append the local file to the ftp file + { + std::string curl_command = "curl -S -s -T \"" + local_file.string() + "\" \"ftp://localhost:2121/hello_world.txt\" --append"; + auto curl_result = std::system(curl_command.c_str()); + + // Make sure that the upload was successful + ASSERT_EQ(curl_result, 0); + + // Make sure that the file exists in the FTP root dir + ASSERT_TRUE(std::filesystem::exists(ftp_file)); + ASSERT_TRUE(std::filesystem::is_regular_file(ftp_file)); + + // Make sure that the file has the same content + std::ifstream ifs(ftp_file.string()); + std::string content((std::istreambuf_iterator(ifs)), + (std::istreambuf_iterator())); + + ASSERT_EQ(content, "HELLO WORLDHello World"); + } + + // Stop the server + server.stop(); +} +#endif + +#if 1 +// According to the RFC 959, a normal upload (STOR) must replace an already existing file +TEST(FineFTPTest, ReplaceFile) { + auto test_working_dir = std::filesystem::current_path(); + auto ftp_root_dir = test_working_dir / "ftp_root"; + auto local_root_dir = test_working_dir / "local_root"; + + { + if (std::filesystem::exists(ftp_root_dir)) + std::filesystem::remove_all(ftp_root_dir); + + if (std::filesystem::exists(local_root_dir)) + std::filesystem::remove_all(local_root_dir); + + // Make sure that we start clean, so no old dir exists + ASSERT_FALSE(std::filesystem::exists(ftp_root_dir)); + ASSERT_FALSE(std::filesystem::exists(local_root_dir)); + + std::filesystem::create_directory(ftp_root_dir); + std::filesystem::create_directory(local_root_dir); + + // Make sure that we were able to create the dir + ASSERT_TRUE(std::filesystem::is_directory(ftp_root_dir)); + ASSERT_TRUE(std::filesystem::is_directory(local_root_dir)); + } + + fineftp::FtpServer server(2121); + server.start(1); + + server.addUserAnonymous(ftp_root_dir.string(), fineftp::Permission::All); + + // Create a hello world.txt file in the local root dir and write "Hello World" into it + auto local_file = local_root_dir / "hello_world.txt"; + { + std::ofstream ofs(local_file.string()); + ofs << "Hello World"; + ofs.close(); + } + + // Make sure that the file exists + ASSERT_TRUE(std::filesystem::exists(local_file)); + ASSERT_TRUE(std::filesystem::is_regular_file(local_file)); + + // create a hello world.txt file in the ftp root dir and write "HELLO WORLD" into it + auto ftp_file = ftp_root_dir / "hello_world.txt"; + { + std::ofstream ofs(ftp_file.string()); + ofs << "HELLO WORLD"; + ofs.close(); + } + + // Make sure that the file exists + ASSERT_TRUE(std::filesystem::exists(ftp_file)); + ASSERT_TRUE(std::filesystem::is_regular_file(ftp_file)); + + // Replace the FTP file with the local file + { + std::string curl_command = "curl -S -s -T \"" + local_file.string() + "\" \"ftp://localhost:2121/hello_world.txt\""; + auto curl_result = std::system(curl_command.c_str()); + + // Make sure that the upload was successful + ASSERT_EQ(curl_result, 0); + + // Make sure that the file exists in the FTP root dir + ASSERT_TRUE(std::filesystem::exists(ftp_file)); + ASSERT_TRUE(std::filesystem::is_regular_file(ftp_file)); + + // Make sure that the file has the same content + std::ifstream ifs(ftp_file.string()); + std::string content((std::istreambuf_iterator(ifs)), + (std::istreambuf_iterator())); + + ASSERT_EQ(content, "Hello World"); + } + + // Stop the server + server.stop(); +} +#endif + #if 0 TEST(FineFTPTest, PathVulnerability) {