BIP32: CExtKey::SetSeed missing validation of seed bit-length #35308

Current behaviour

CExtKey::SetSeed does not check the seed bit-length constraints defined in BIP32

Generate a seed byte sequence S of a chosen length (between 128 and 512 bits; 256 bits is advised) from a (P)RNG.

I don't see a reason for not adhering to the spec in this case. Weak seeds used for master key generation can & have been exploited.

Found using the bitcoinfuzz tool.

Expected behaviour

CExtKey::SetSeed should provide bit-length (or byte-length) checks for the parameter seed e.g.,

void CExtKey::SetSeed(std::span<const std::byte> seed)
{
    if (seed.size() < 16) return; // or throw
    ...
}

Steps to reproduce

Added a testcase in key_tests.cpp and ran the tests:

BOOST_AUTO_TEST_CASE(extkey_setseed_empty)
{
    CExtKey key;
    
    // Test with 0 bytes
    std::vector<std::byte> empty{};
    key.SetSeed(empty);

    // Test with 1 byte
    std::vector<std::byte> one{std::byte{0x01}};
    key.SetSeed(one);
    
}

Running 8 test cases...

*** No errors detected

How did you obtain Bitcoin Core

Compiled from source

What version of Bitcoin Core are you using?

master@ed1795a

Operating system and version

Ubuntu 24.04.3 LTS

The reference about exploiting weak seeds is behind an authentication wall. Could you provide a public link for it?

edit: I found it: https://is.muni.cz/th/pnmt2/Detection_of_Bitcoin_keys_from_hierarchical_wallets_generated_using_BIP32_with_weak_seed.pdf

The attacks in the cited paper are about seeds with weak entropy content, not short seeds, so length alone is a poor proxy for the vulnerability described.

That said, if a length check is added for spec compliance, I believe the correct approach would be to add Assert(seed.size() >= 16 && seed.size() <= 64), since all production callers already pass a validated 32-byte seed it should not be a problem.

The attacks in the cited paper are about seeds with weak entropy content, not short seeds, so length alone is a poor proxy for the vulnerability described.

I would argue that being short qualifies seeds as weak, meaning they are easier to brute force.

I would argue that being short qualifies seeds as weak, meaning they are easier to brute force.

Yes, but a short seed can still be stronger than a big seed filled by only 0 bits. What I meant is that just checking seed length alone there would be for BIP32 spec compliance and not for preventing weak seeds, because you need to verify entropy quality before generation, and Core already does that on startup Random_SanityCheck.