What is an uninitialized read? Can you point out some examples found in Bitcoin Core? How do you detect them?

Question

What is an uninitialized read? Can you point out some examples found in Bitcoin Core? How do you detect them?

Answer 1

An uninitialized read is when a variable is created without an initial value and then the value of this variable is read. This should be avoided as the resulting behavior is unpredictable and could be different each time the program is run. According to cppreference.com:

Normally uninitialized variables are a bad idea, and the only place where they are useful is when you are about to read the variable in from some input stream.

There are various tools for detecting uninitialized reads, some of which were covered in this Bitcoin Core PR review club session in December 2019.

These include dynamic analysis tools such as MemorySanitizer (MSan), Valgrind (specifically Memcheck) and static analysis tools that examine the code without running it.

Uninitialized reads can also be found through unit, functional or fuzz tests, by compiling with the -Werror=uninitialized flag or compiling with Clang with the -ftrivial-auto-var-init=pattern flag which will pre-initialize variables with dummy values.

History of detected uninitialized reads in Bitcoin Core

The most recently detected uninitialized read at the time of writing (October 2020) was found by Marco Falke in the recently merged Signet code. He found it through visual review and it was caught before it made it into a major Bitcoin Core release.

As practicalswift highlights, there have been a number of examples of uninitialized reads found in recent years but with more widespread usage of sophisticated tools these are likely to be found with decreasing frequency.

2020: Use of uninitialized memory in Erlay networking code - found pre-merge

2020: Use of uninitialized memory in BIP324 encrypted p2p transport de-/serializer code (truth in advertising: I haven't verified this one by writing a PoC) - found pre-merge

2020: util: Avoid potential uninitialized read in FormatISO8601DateTime(int64_t) by checking gmtime_s/gmtime_r return

2019: Use of uninitialized memory in networking code when receiving a transaction we already have - found post-merge

2019: wallet: Uninitialized read in bumpfee(…)

2018: wallet: Fix non-determinism in ParseHDKeypath(...). Avoid using an uninitialized variable in path calculation.

2018: wallet: Fix use of uninitialized value bnb_used in CWallet::CreateTransaction(...)

2017: [net] Fix use of uninitialized value in getnetworkinfo(const JSONRPCRequest&)

2017: [test] Avoid reading a potentially uninitialized variable in tx_invalid-test (transaction_tests.cpp)