From 9cb31326e3c4f546752f6e42dbf0eedcb0260627 Mon Sep 17 00:00:00 2001 From: DanS Date: Fri, 16 Oct 2026 20:44:00 -0500 Subject: [PATCH] main: open block and undo files read-only when they're opened for reading OpenDiskFile opened every block or undo file "rb+", even for its fReadOnly callers, so every reader held a writable fd. That's what let the wolfSSL CloseSocket bug corrupt blocks/*.dat. A P2P double close took the wallet's BDB fd away; the number was then handed to a block file opened for a getblock/getdata read; BDB's next wallet page write landed in it, at offset pgno*4096. On production that destroyed blocks 2,728,132-2,728,161 in blk00011.dat and block 3,356,880 in blk00020.dat. Every fReadOnly caller only reads through the handle: - ReadBlockFromDisk, UndoReadFromDisk, both GetTransaction paths and hush_blockload read via CAutoFile >>; - the reindex loop hands it to LoadExternalBlockFile, which reads via CBufferedFile; - the existence check at startup only opens it; - hush_prefetch only freads. Open those "rb". A stray write through a reused fd number then fails (EBADF) instead of silently corrupting a block file. Missing files still fail to open as before; writers keep "rb+"/"wb+". FdDefense.ReadOnlyBlockFileIsNotWritable: pwrite through a read-only block file's fd fails with EBADF, while the writable open still writes. Co-Authored-By: Claude Opus 5.5 --- src/Makefile.gtest.include | 3 +- src/gtest/test_fd_defense.cpp | 55 +++++++++++++++++++++++++++++++++++ src/main.cpp | 5 +++- 3 files changed, 61 insertions(+), 2 deletions(-) create mode 100644 src/gtest/test_fd_defense.cpp diff --git a/src/Makefile.gtest.include b/src/Makefile.gtest.include index a79fec340..35042a0b7 100644 --- a/src/Makefile.gtest.include +++ b/src/Makefile.gtest.include @@ -16,7 +16,8 @@ hush_gtest_SOURCES = \ gtest/test_mnemonic_compat.cpp \ gtest/test_stratum_jobid.cpp \ gtest/test_wallet_notedata.cpp \ - gtest/test_net_closesocket.cpp + gtest/test_net_closesocket.cpp \ + gtest/test_fd_defense.cpp hush_gtest_CPPFLAGS = $(AM_CPPFLAGS) -DMULTICORE -fopenmp -DBINARY_OUTPUT -DCURVE_ALT_BN128 -DSTATIC $(BITCOIN_INCLUDES) hush_gtest_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS) diff --git a/src/gtest/test_fd_defense.cpp b/src/gtest/test_fd_defense.cpp new file mode 100644 index 000000000..20c754a2a --- /dev/null +++ b/src/gtest/test_fd_defense.cpp @@ -0,0 +1,55 @@ +// Copyright (c) 2016-2024 The Hush developers +// Distributed under the GPLv3 software license, see the accompanying +// file COPYING or https://www.gnu.org/licenses/gpl-3.0.en.html + +#include + +#include +#include +#include + +#include + +#include "chainparams.h" +#include "main.h" +#include "util.h" + +// A block file opened for reading must not be writable through its fd: a stray write through a reused fd number +// (the wolfSSL CloseSocket macro let the wallet's BDB fd be closed behind its back) used to land in it. +TEST(FdDefense, ReadOnlyBlockFileIsNotWritable) { + SelectParams(CBaseChainParams::MAIN); + const boost::filesystem::path dir = boost::filesystem::temp_directory_path() / boost::filesystem::unique_path("fddefense-%%%%%%%%"); + const bool hadDatadir = mapArgs.count("-datadir") > 0; + const std::string oldDatadir = hadDatadir ? mapArgs["-datadir"] : ""; + boost::filesystem::create_directories(dir); + mapArgs["-datadir"] = dir.string(); + ClearDatadirCache(); + + const CDiskBlockPos pos(0, 0); + const boost::filesystem::path blk = GetBlockPosFilename(pos, "blk"); + boost::filesystem::create_directories(blk.parent_path()); + { + FILE* f = fopen(blk.string().c_str(), "wb"); + ASSERT_TRUE(f != nullptr); + fputs("block bytes", f); + fclose(f); + } + + FILE* ro = OpenBlockFile(pos, true); + ASSERT_TRUE(ro != nullptr); + char c = 0; + EXPECT_EQ(1u, fread(&c, 1, 1, ro)) << "still readable"; + errno = 0; + EXPECT_EQ(-1, pwrite(fileno(ro), "X", 1, 0)) << "a write through a read-only block file's fd must fail"; + EXPECT_EQ(EBADF, errno); + fclose(ro); + + FILE* rw = OpenBlockFile(pos, false); + ASSERT_TRUE(rw != nullptr); + EXPECT_EQ(1, pwrite(fileno(rw), "b", 1, 0)) << "the writable open still writes"; + fclose(rw); + + if (hadDatadir) mapArgs["-datadir"] = oldDatadir; else mapArgs.erase("-datadir"); + ClearDatadirCache(); + boost::filesystem::remove_all(dir); +} diff --git a/src/main.cpp b/src/main.cpp index 492d0b66b..db0af063b 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -5922,7 +5922,10 @@ FILE* OpenDiskFile(const CDiskBlockPos &pos, const char *prefix, bool fReadOnly) return NULL; boost::filesystem::path path = GetBlockPosFilename(pos, prefix); boost::filesystem::create_directories(path.parent_path()); - FILE* file = fopen(path.string().c_str(), "rb+"); + // Read-only opens get a read-only FILE (every fReadOnly caller only reads). With "rb+", a write through a + // reused fd number landed here: when the wolfSSL CloseSocket macro (see net.h) closed the wallet's BDB fd + // behind its back, BDB's next page write went to whatever reopened that number -- a block file opened here. + FILE* file = fopen(path.string().c_str(), fReadOnly ? "rb" : "rb+"); if (!file && !fReadOnly) file = fopen(path.string().c_str(), "wb+"); if (!file) {