dbwrapper: use RAII for Logv heap buffer and handle vsnprintf failure #35347

dxbjavid commented at 4:44 PM on May 21, 2026: none

What

CBitcoinLevelDBLogger::Logv in src/dbwrapper.cpp allocates its 30 KiB fallback buffer with a raw new char[] and frees it with a matching delete[] at the bottom of the loop. This change replaces the raw new[]/delete[] pair with a std::unique_ptr<char[]> and clamps negative vsnprintf returns to zero so the rest of the function cannot perform negative pointer arithmetic on base.

Why

Two small issues with the current code:

Exception leak. Any exception thrown between the new char[30000] and the matching delete[] base leaks the buffer. The LogDebug(...) call near the end of the loop dispatches through tfm::format and the logging backend, both of which can throw std::bad_alloc. The if (base != buffer) delete[] base; cleanup is unreachable in that case.
vsnprintf negative return. The loop adds the return value of vsnprintf to a char* (p += vsnprintf(...)). vsnprintf may return a negative value on encoding errors. When that happens, p ends up pointing before the beginning of base, after which p[-1] (the trailing-newline check, current line 100) is an out-of-bounds read and the subsequent base[std::min(bufsize - 1, (int)(p - base))] = '\0' writes through a negative index.

What changed

The heap buffer is now owned by std::unique_ptr<char[]> allocated via std::make_unique. The buffer is released automatically on every exit path of the loop, including exception unwinding.
The result of vsnprintf is read into a local int and p is only advanced if it is positive, so subsequent p[-1] reads and (int)(p - base) arithmetic stay within base.
Behaviour in the normal case (successful format, no exception) is unchanged.

Test plan

cmake --build build --target bitcoin_node succeeds with the patch applied.
No behaviour change for callers; this is purely a local cleanup of one logger function.

dbwrapper: use RAII for Logv heap buffer and handle vsnprintf failure

CBitcoinLevelDBLogger::Logv allocates its 30 KiB fallback buffer with a
raw `new char[]` and frees it with a matching `delete[]` at the bottom
of the loop. Any exception thrown between the allocation and the
`delete[]` leaks the buffer; in particular the `LogDebug(...)` call near
the end of the loop dispatches through `tfm::format` and the logging
backend, both of which can throw `std::bad_alloc`.

The same loop also adds the return value of `vsnprintf` to a `char*`
without checking for errors. `vsnprintf` returns a negative value on
encoding errors. When that happens `p` ends up pointing before the
beginning of `base`, after which `p[-1]` (line 100, the trailing-newline
check) is an out-of-bounds read and the subsequent
`base[std::min(bufsize - 1, (int)(p - base))] = '\0'` writes through a
negative index.

Replace the raw `new[]`/`delete[]` with a `std::unique_ptr<char[]>`
allocated via `std::make_unique`, and clamp negative `vsnprintf` returns
to zero before advancing `p`. Behaviour is unchanged in the normal case;
the heap allocation is now released automatically on every exit path,
and the negative-return code path can no longer perform OOB pointer
arithmetic on `base`.

Compiled with the existing build (cmake --build build --target
bitcoin_node) to confirm the translation unit still builds.

1b3e3f4ede

DrahtBot commented at 4:44 PM on May 21, 2026: contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/35347.

Reviews

See the guideline for information on the review process. A summary of reviews will appear here.

sedited commented at 9:14 PM on May 21, 2026: contributor

I don't think this patch solves a clear problem that is worth the review cycles for this project and I'm also not sure if this patch is actually correct. The motivation for the change seems weak to me, we have many other places where a bad_alloc exception would lead to an inconsistent state. Since review cycles are the scarcest resource of this project, I'm going to close this PR again.

sedited closed this on May 21, 2026