What the developer wrote
Authored by stickies-v
kernel: allow null data_directory
An empty path may be represented with a nullptr. For example,
std::string_view::data() may return nullptr.
Removes the BITCOINKERNEL_ARG_NONNULL attribute for data_directory,
and instead handles such null arguments in the implementation.
Also documents how BITCOINKERNEL_ARG_NONNULL should be used.
What changed, and why it matters
This commit tightens input handling in Bitcoin Core's kernel library. Previously, the library's C interface promised that callers must not pass a null or empty directory path, but it did not actually check. Now it explicitly rejects null or empty data and blocks directories and returns an error instead of potentially misbehaving. It also updates the internal C++ wrapper to accept string views, which can legitimately have null data pointers when empty. This is a defensive hardening change, not a fix for an active exploit.
No immediate action required beyond normal review and merge. Users of the kernel library should ensure they pass non-null, non-empty directory strings. Downstream integrators using the C++ wrapper should note it now accepts std::string_view.
Security signals we found
Defensive null/empty input validation added to C API
Nonnull attribute narrowed to avoid undefined behavior on valid empty inputs
Documentation clarifies correct use of nonnull annotations
Unit tests added for null/empty directory arguments
Evidence from the diff
The patch removes BITCOINKERNEL_ARG_NONNULL(2) from btck_chainstate_manager_options_create and adds an explicit null/empty check for data_dir and blocks_dir, returning nullptr and logging an error. It documents that BITCOINKERNEL_ARG_NONNULL should be reserved for opaque handles where null is unambiguously a programmer error, not for raw data pointers that may be null from empty spans or string_views. The C++ wrapper changes the ChainstateManagerOptions constructor parameters from const std::string& to std::string_view and passes data()/length() directly. Tests are added verifying that empty or null-backed string_views throw std::runtime_error via the wrapper.
Changed components
src/kernel/bitcoinkernel.cppsrc/kernel/bitcoinkernel.hsrc/kernel/bitcoinkernel_wrapper.hsrc/test/kernel/test_kernel.cppInspect captured patch +39 / −8
diff --git a/src/kernel/bitcoinkernel.cpp b/src/kernel/bitcoinkernel.cpp
index 0b9ea513..caa1b6ec 100644
--- a/src/kernel/bitcoinkernel.cpp
+++ b/src/kernel/bitcoinkernel.cpp
@@ -896,6 +896,10 @@ btck_BlockValidationResult btck_block_validation_state_get_block_validation_resu
btck_ChainstateManagerOptions* btck_chainstate_manager_options_create(const btck_Context* context, const char* data_dir, size_t data_dir_len, const char* blocks_dir, size_t blocks_dir_len)
{
+ if (data_dir == nullptr || data_dir_len == 0 || blocks_dir == nullptr || blocks_dir_len == 0) {
+ LogError("Failed to create chainstate manager options: dir must be non-null and non-empty");
+ return nullptr;
+ }
try {
fs::path abs_data_dir{fs::absolute(fs::PathFromString({data_dir, data_dir_len}))};
fs::create_directories(abs_data_dir);
diff --git a/src/kernel/bitcoinkernel.h b/src/kernel/bitcoinkernel.h
index 93e160cd..14f7c7b1 100644
--- a/src/kernel/bitcoinkernel.h
+++ b/src/kernel/bitcoinkernel.h
@@ -35,6 +35,17 @@
#else
#define BITCOINKERNEL_WARN_UNUSED_RESULT
#endif
+
+/**
+ * BITCOINKERNEL_ARG_NONNULL is a compiler attribute used to indicate that
+ * certain pointer arguments to a function are not expected to be null.
+ *
+ * Callers must not pass a null pointer for arguments marked with this attribute,
+ * as doing so may result in undefined behavior. This attribute should only be
+ * used for arguments where a null pointer is unambiguously a programmer error,
+ * such as for opaque handles, and not for pointers to raw input data that might
+ * validly be null (e.g., from an empty std::span or std::string).
+ */
#if !defined(BITCOINKERNEL_BUILD) && defined(__GNUC__)
#define BITCOINKERNEL_ARG_NONNULL(...) __attribute__((__nonnull__(__VA_ARGS__)))
#else
@@ -933,11 +944,12 @@ BITCOINKERNEL_API const btck_BlockHash* BITCOINKERNEL_WARN_UNUSED_RESULT btck_bl
* @brief Create options for the chainstate manager.
*
* @param[in] context Non-null, the created options and through it the chainstate manager will
- associate with this kernel context for the duration of their lifetimes.
- * @param[in] data_directory Non-null, path string of the directory containing the chainstate data.
- * If the directory does not exist yet, it will be created.
- * @param[in] blocks_directory Non-null, path string of the directory containing the block data. If
- * the directory does not exist yet, it will be created.
+ * associate with this kernel context for the duration of their lifetimes.
+ * @param[in] data_directory Non-null, non-empty path string of the directory containing the
+ * chainstate data. If the directory does not exist yet, it will be
+ * created.
+ * @param[in] blocks_directory Non-null, non-empty path string of the directory containing the block
+ * data. If the directory does not exist yet, it will be created.
* @return The allocated chainstate manager options, or null on error.
*/
BITCOINKERNEL_API btck_ChainstateManagerOptions* BITCOINKERNEL_WARN_UNUSED_RESULT btck_chainstate_manager_options_create(
@@ -945,7 +957,7 @@ BITCOINKERNEL_API btck_ChainstateManagerOptions* BITCOINKERNEL_WARN_UNUSED_RESUL
const char* data_directory,
size_t data_directory_len,
const char* blocks_directory,
- size_t blocks_directory_len) BITCOINKERNEL_ARG_NONNULL(1, 2);
+ size_t blocks_directory_len) BITCOINKERNEL_ARG_NONNULL(1);
/**
* @brief Set the number of available worker threads used during validation.
diff --git a/src/kernel/bitcoinkernel_wrapper.h b/src/kernel/bitcoinkernel_wrapper.h
index b847dde5..662f3724 100644
--- a/src/kernel/bitcoinkernel_wrapper.h
+++ b/src/kernel/bitcoinkernel_wrapper.h
@@ -937,8 +937,9 @@ public:
class ChainstateManagerOptions : public UniqueHandle<btck_ChainstateManagerOptions, btck_chainstate_manager_options_destroy>
{
public:
- ChainstateManagerOptions(const Context& context, const std::string& data_dir, const std::string& blocks_dir)
- : UniqueHandle{btck_chainstate_manager_options_create(context.get(), data_dir.c_str(), data_dir.length(), blocks_dir.c_str(), blocks_dir.length())}
+ ChainstateManagerOptions(const Context& context, std::string_view data_dir, std::string_view blocks_dir)
+ : UniqueHandle{btck_chainstate_manager_options_create(
+ context.get(), data_dir.data(), data_dir.length(), blocks_dir.data(), blocks_dir.length())}
{
}
diff --git a/src/test/kernel/test_kernel.cpp b/src/test/kernel/test_kernel.cpp
index 75c9e466..b46d39ad 100644
--- a/src/test/kernel/test_kernel.cpp
+++ b/src/test/kernel/test_kernel.cpp
@@ -625,6 +625,20 @@ BOOST_AUTO_TEST_CASE(btck_chainman_tests)
ChainstateManagerOptions chainman_opts{context, test_directory.m_directory.string(), (test_directory.m_directory / "blocks").string()};
ChainMan chainman{context, chainman_opts};
}
+ { // null or empty data_directory or blocks_directory are not allowed
+ Context context{};
+ auto valid_dir{test_directory.m_directory.string()};
+ std::vector<std::pair<std::string_view, std::string_view>> illegal_cases{
+ {"", valid_dir},
+ {valid_dir, {nullptr, 0}},
+ {"", ""},
+ {{nullptr, 0}, {nullptr, 0}},
+ };
+ for (auto& [data_dir, blocks_dir] : illegal_cases) {
+ BOOST_CHECK_THROW(ChainstateManagerOptions(context, data_dir, blocks_dir),
+ std::runtime_error);
+ };
+ }
auto notifications{std::make_shared<TestKernelNotifications>()};
auto context{create_context(notifications, ChainType::MAINNET)};
Why this scored 26/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.