From 01678166a8dae6c08c334a8fae0894cbdd51b749 Mon Sep 17 00:00:00 2001 From: dxbjavid Date: Tue, 9 Jun 2026 09:53:57 +0530 Subject: [PATCH] fileutils: force owner write while copying read-only sources Opening the destination at the source's exact mode breaks when the source lacks owner write (e.g. r-xr-xr-x): the create still works but the overwrite path can't reopen such a destination for writing. Force owner read+write while copying and let the trailing Chmod() put the source mode back, which only ever adds owner bits so the group/other bits stay as restrictive as the source throughout. Add a regression test covering the restricted-mode and read-only-source cases. --- src/FileUtils.cpp | 6 ++++- test/UtilsTest.cpp | 55 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 60 insertions(+), 1 deletion(-) diff --git a/src/FileUtils.cpp b/src/FileUtils.cpp index bf434431..c11306c5 100644 --- a/src/FileUtils.cpp +++ b/src/FileUtils.cpp @@ -265,7 +265,11 @@ bool CFile::Copy(const CString& sOldFileName, const CString& sNewFileName, iMode = st.st_mode & 07777; } - if (!NewFile.Open(O_WRONLY | O_CREAT | O_TRUNC, iMode)) { + // Force owner read+write while copying so the write still works for a + // source that lacks them (e.g. r-xr-xr-x); the trailing Chmod() puts + // the exact source mode back. This only ever adds owner bits, so the + // group/other bits stay as restrictive as the source the whole time. + if (!NewFile.Open(O_WRONLY | O_CREAT | O_TRUNC, iMode | S_IRUSR | S_IWUSR)) { return false; } diff --git a/test/UtilsTest.cpp b/test/UtilsTest.cpp index 61c016b3..816ac786 100644 --- a/test/UtilsTest.cpp +++ b/test/UtilsTest.cpp @@ -18,6 +18,61 @@ #include #include +namespace { +CString WriteTempFile(const CString& sContent, mode_t iMode) { + char sName[] = "./copytest-XXXXXX"; + int fd = mkstemp(sName); + EXPECT_NE(fd, -1); + close(fd); + + CFile File(sName); + EXPECT_TRUE(File.Open(O_WRONLY | O_TRUNC)); + File.Write(sContent); + File.Close(); + EXPECT_TRUE(CFile::Chmod(sName, iMode)); + return sName; +} + +unsigned ModeOf(const CString& sFile) { + struct stat st; + EXPECT_EQ(CFile::GetInfo(sFile, st), 0); + return st.st_mode & 07777; +} +} // namespace + +// A 0600 source must never widen to the default 0644 during the copy, so the +// destination ends up restricted too. +TEST(FileUtilsTest, CopyKeepsRestrictiveMode) { + CString sSrc = WriteTempFile("secret", 0600); + CString sDst = sSrc + "-copy"; + + EXPECT_TRUE(CFile::Copy(sSrc, sDst)); + EXPECT_EQ(ModeOf(sDst), 0600u); + + CFile::Delete(sSrc); + CFile::Delete(sDst); +} + +// A source the owner can't write to (r-xr-xr-x) must still copy and keep its +// mode; the copy forces owner write internally and chmods it back afterwards. +TEST(FileUtilsTest, CopyReadOnlySource) { + CString sSrc = WriteTempFile("public", 0555); + CString sDst = sSrc + "-copy"; + + EXPECT_TRUE(CFile::Copy(sSrc, sDst)); + EXPECT_EQ(ModeOf(sDst), 0555u); + + CString sContent; + CFile Dst(sDst); + ASSERT_TRUE(Dst.Open()); + Dst.ReadFile(sContent); + Dst.Close(); + EXPECT_EQ(sContent, "public"); + + CFile::Delete(sSrc); + CFile::Delete(sDst); +} + TEST(IRC32, GetMessageTags) { EXPECT_EQ(CUtils::GetMessageTags(""), MCString()); EXPECT_EQ(CUtils::GetMessageTags(