bitcoin / bitcoin/bitcoin

txdb: Cursor warmup accepts malformed first coin key

Open Beginner friendly
#35,172 4 comments 0 reactions 0 assignees View on GitHub
Bug Data corruption
Dominant language
C++
Stars
90.2k
Forks
39.4k
Avg merge
3d 8h
Merged PRs (30d)
88

Description

### Is there an existing issue for this?

- [x] I have searched the existing issues

### Current behaviour

`CCoinsViewDB::Cursor()` primes the first cached key without checking whether `pcursor->GetKey(entry)` succeeded. When the first record in the `DB_COIN` keyspace cannot be decoded as `CoinEntry`, `entry.key` stays at its default `DB_COIN` value from `CoinEntry { uint8_t key{DB_COIN}; ... }`, so the returned cursor reports `Valid()==true` and `GetKey()==true`.

At `859215218667ca9f35d5adae0289e4a125798087`, `src/txdb.cpp` does this in the constructor warmup:

```cpp
if (i->pcursor->Valid()) {
CoinEntry entry(&i->keyTmp.second);
i->pcursor->GetKey(entry);
i->keyTmp.first = entry.key;
}
```

The sibling path in `CCoinsViewDBCursor::Next()` already handles the same failure correctly, as fixed in `#7890` (`a3310b4d48`):

```cpp
if (!pcursor->Valid() || !pcursor->GetKey(entry)) {
keyTmp.first = 0;
} else {
keyTmp.first = entry.key;
}
```

This requires an already malformed chainstate keyspace, such as manual DB modification or database corruption that leaves a readable LevelDB record. `Cursor()` is used by UTXO stats, snapshot export, `scantxoutset`, and a rollback-copy path, not by consensus or wallet code. With a malformed first key whose value still deserializes as `Coin`, those paths can silently process a bogus first entry instead of rejecting it.

### Expected behaviour

`Cursor()` should mirror `Next()`: after `Seek(DB_COIN)`, if `!i->pcursor->Valid()` or `!i->pcursor->GetKey(entry)`, it should set `i->keyTmp.first = 0`; otherwise it should cache `entry.key`.

A malformed first `DB_COIN` record should leave the returned cursor invalid, so `cursor->Valid()` and `cursor->GetKey(outpoint)` both return `false`.

### Steps to reproduce

Add a unit test that writes a malformed first `DB_COIN` key into a real `CDBWrapper`, reopens the same path via `CCoinsViewDB`, and checks the returned cursor. The key `uint8_t{'C'}` is enough to enter the coin keyspace but is too short to decode as `CoinEntry`.

Minimal test case:

```cpp
BOOST_AUTO_TEST_CASE(malformed_first_coin_key_cursor_invalid)
{
const fs::path path{m_args.GetDataDirBase() / "malformed_first_coin_key_cursor_invalid"};

{
CDBWrapper dbw({.path = path, .cache_bytes = 1_MiB, .wipe_data = true, .obfuscate = false});
dbw.Write(uint8_t{'C'}, Coin{CTxOut{1, CScript{}}, 1, false});
}

CCoinsViewDB view({.path = path, .cache_bytes = 1_MiB, .wipe_data = false, .obfuscate = false}, {});
std::unique_ptr cursor{view.Cursor()};
BOOST_REQUIRE(cursor);

COutPoint outpoint;
BOOST_CHECK(!cursor->Valid());
BOOST_CHECK(!cursor->GetKey(outpoint));
}
```

After registering the test in `src/test/CMakeLists.txt`, build and run:

```bash
cmake -B build -GNinja -DBUILD_TESTS=ON -DENABLE_WALLET=OFF -DWITH_MINIUPNPC=OFF -DWITH_ZMQ=OFF -DCMAKE_BUILD_TYPE=Debug
cmake --build build --target test_bitcoin -j$(sysctl -n hw.ncpu)
./build/bin/test_bitcoin --run_test=txdb_cursor_tests
```

The positive control `valid_first_coin_key_cursor_valid`, using a normal `CCoinsViewCache::AddCoin()` entry as the first key, passes cleanly.

### Relevant log output

```text
Running 2 test cases...
test/txdb_cursor_tests.cpp:42: error: in "txdb_cursor_tests/malformed_first_coin_key_cursor_invalid": check !cursor->Valid() has failed
test/txdb_cursor_tests.cpp:43: error: in "txdb_cursor_tests/malformed_first_coin_key_cursor_invalid": check !cursor->GetKey(outpoint) has failed

*** 2 failures are detected in the test module "Bitcoin Core Test Suite"
```

Positive control:

```text
Running 1 test case...

*** No errors detected
```

### How did you obtain Bitcoin Core

Compiled from source

### What version of Bitcoin Core are you using?

master@859215218667ca9f35d5adae0289e4a125798087

### Operating system and version

macOS 26.4.1

### Machine specifications

_No response_

Contributor guide

Open the contributing guide

Research direction

Start in src/txdb.cpp at CCoinsViewDB::Cursor() and compare its warmup logic with CCoinsViewDBCursor::Next(). Add the malformed-first-key and positive-control cases in src/test/txdb_cursor_tests.cpp, register the test in src/test/CMakeLists.txt, then run the txdb_cursor_tests target. Done means the malformed cursor is invalid while the valid control remains valid.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
databases
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
78/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.