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 <noreply@anthropic.com>
This commit is contained in:
@@ -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)
|
||||
|
||||
@@ -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 <gtest/gtest.h>
|
||||
|
||||
#include <errno.h>
|
||||
#include <stdio.h>
|
||||
#include <unistd.h>
|
||||
|
||||
#include <boost/filesystem.hpp>
|
||||
|
||||
#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);
|
||||
}
|
||||
+4
-1
@@ -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) {
|
||||
|
||||
Reference in New Issue
Block a user