From aa99376370f89111c18d3c893f51a0537cc6be7c Mon Sep 17 00:00:00 2001 From: Victor Zverovich Date: Sun, 8 Nov 2026 08:04:42 -0800 Subject: [PATCH] Don't corrupt the FILE buffer when printing to a non-writable stream init_buffer decremented the write pointer even when putc_unlocked failed, turning a null pointer into (char*)-1 that print then wrote through, e.g. when printing to a stream opened for reading. Report an error instead. --- include/fmt/format-inl.h | 38 +++++++++++++++++++++++++------------- test/format-test.cc | 7 +++++++ 2 files changed, 32 insertions(+), 13 deletions(-) diff --git a/include/fmt/format-inl.h b/include/fmt/format-inl.h index a55c3cd0..36087a93 100644 --- a/include/fmt/format-inl.h +++ b/include/fmt/format-inl.h @@ -1521,11 +1521,14 @@ template class glibc_file : public file_base { return (this->file_->_flags & unbuffered) == 0; } - void init_buffer() { - if (this->file_->_IO_write_ptr < this->file_->_IO_write_end) return; + // Returns false if the buffer cannot be initialized, e.g. because the file + // is not open for writing. + auto init_buffer() -> bool { + if (this->file_->_IO_write_ptr < this->file_->_IO_write_end) return true; // Force buffer initialization by placing and removing a char in a buffer. - putc_unlocked(0, this->file_); + if (putc_unlocked(0, this->file_) == EOF) return false; --this->file_->_IO_write_ptr; + return true; } // Returns the file's read buffer. @@ -1570,12 +1573,13 @@ template class apple_file : public file_base { return (this->file_->_flags & unbuffered) == 0; } - void init_buffer() { - if (this->file_->_p) return; + auto init_buffer() -> bool { + if (this->file_->_p) return true; // Force buffer initialization by placing and removing a char in a buffer. - putc_unlocked(0, this->file_); + if (putc_unlocked(0, this->file_) == EOF) return false; --this->file_->_p; ++this->file_->_w; + return true; } auto get_read_buffer() const -> span { @@ -1612,7 +1616,7 @@ template class fallback_file : public file_base { auto is_buffered() const -> bool { return false; } auto needs_flush() const -> bool { return false; } - void init_buffer() {} + auto init_buffer() -> bool { return true; } auto get_read_buffer() const -> span { return {&next_, has_next_ ? 1u : 0u}; @@ -1665,6 +1669,14 @@ class file_print_buffer::value>> private: file_ref file_; + void unlock() { + F* f = file_; // Make funlockfile depend on the template parameter F. +#ifdef __SANITIZE_THREAD__ + __tsan_release(f); +#endif + funlockfile(f); // for the system API detection to work. + } + static void grow(buffer& base, size_t) { auto& self = static_cast(base); self.file_.advance_write_buffer(self.size()); @@ -1681,18 +1693,18 @@ class file_print_buffer::value>> #ifdef __SANITIZE_THREAD__ __tsan_acquire(f); #endif - file_.init_buffer(); + if (!file_.init_buffer()) { + int error_code = errno; + unlock(); + FMT_THROW(system_error(error_code, FMT_STRING("cannot write to file"))); + } auto buf = file_.get_write_buffer(); set(buf.data, buf.size); } ~file_print_buffer() { file_.advance_write_buffer(size()); bool flush = file_.needs_flush(); - F* f = file_; // Make funlockfile depend on the template parameter F. -#ifdef __SANITIZE_THREAD__ - __tsan_release(f); -#endif - funlockfile(f); // for the system API detection to work. + unlock(); if (flush) fflush(file_); } }; diff --git a/test/format-test.cc b/test/format-test.cc index 19f1fd52..79659d9f 100644 --- a/test/format-test.cc +++ b/test/format-test.cc @@ -2875,6 +2875,13 @@ TEST(format_test, print_to_broken_pipe) { } std::signal(SIGPIPE, old_handler); } + +TEST(format_test, print_to_read_only_file) { + auto pipe = fmt::pipe(); + auto read_end = pipe.read_end.fdopen("r"); + EXPECT_THROW(fmt::print(read_end.get(), "{}", std::string(100, 'x')), + std::system_error); +} #endif // FMT_USE_FCNTL // Only defined after the test case.