pull down to refresh

This is a pretty devastating read.

It appears that the developer is naively trying to override the pyb_rng_get_obj variable by creating a duplicate version of the variable. C does not work this way. This error would have given him a compiler error of “duplicate symbol pyb_rng_get_obj, because there are now two places where it is defined: the stm32 library, and in the newly added rng.c file.

From here, I assume in a bout of frustration, he set MICROPY_HW_ENABLE_RNG to 0, which would have resolved the compiler error.

Sometimes when developers are flailing, they’ll try random things to see if it helps. Setting ``MICROPY_HW_ENABLE_RNG` to 0 would have made the compiler error go away for the wrong reason.

This swept the compiler error of having two conflicting definitions under the rug.
The compiler error was begging the programmer to reconsider his logic. Instead of that happening, the compiler error was just silenced. The compiler was giving the developer one last chance to reconsider what he was about to do, but the alarm was ignored and silenced.
You must understand the code you are shipping. Full stop. There are no excuses. Layers of misdirection make it harder to understand.

Nullc left a response to this on Hacker News:

This writeup isn't very good and misses/misunderstands the programming error that leads to the flaw.

I'm commenting because I think it's important to understand the issue.

The article would have you think that the change in question was a tiny change to a flag to make it compile, but in reality the commit in question is a 1533 line addition of the entire RNG infrastructure.

The fundamental cause is a mixup between a value test and a definedness test.

Coldcard attempted to replace the micropython wrapper on the hardware TRNG, apparently in order to provide a more aggressive handling of fault/error conditions.

The micropython hwrng code is gated by an #if check, the replacement HWRNG code is gated by an ifndef. So "#define MICROPY_HW_ENABLE_RNG (0)" deactivated the micropython implementation but failed to activate the internal one (which was #ifndef MICROPY_HW_ENABLE_RNG ... which didn't fire because MICROPY_HW_ENABLE_RNG was defined).

This was easier to miss because the usages weren't only in different files-- they were in different repositories.

There is a more abstract point to make that in cryptographic software the absence of a secure randomness source (the STM32 TRNG) should never fall back to an insecure source (a trivial PRNG which might have only had on the order of 20-bits of uncertainty in its input). But the code that had the fallback was micropython which was not authored by the coldcard creators and is presumably not intended for cryptographic applications...

In later code (for MK4+ devices) the issue was further masked without being corrected by xor-ing in another insecure PRNG seeded by 32-bits from another TRNG. ... itself acting like an additional insecure fallback. (Why it first hashes 64-bits of TRNG output then throws away half the entropy is a mystery...)

RNG failures can be difficult to detect because the real randomness and a PRNG are indistinguishable by any simple tests of the output. I understand the coldcard developers ran extensive tests on the randomness generated by these devices-- they may well have been just testing the PRNG. It's something of a "color of your bits" issue ( https://ansuz.sooke.bc.ca/entry/23 ).

The same sort of issue happens at multiple levels of the stack, e.g. IIRC the STM32 TRNG itself does some kind of whitening that could have the same effect of concealing an RNG failure.

The existence of insecure fake randomness in the code at all was a red flag that had been noted previously -- though the really bad one was hidden away in the micropython code and not even obviously at play. I think this is a rare case of a bug that would be more easily found from binary analysis than review of the source code (e.g. no access to the STM32 TRNG at all).

Without these fallbacks the failure to use the TRNG would have been immediately detected by the developers (e.g. when every attempt generated the same seed), and a review could be validated by fault injection (NOP out the hwrng and verify that the test fails).
reply
106 sats \ 0 replies \ @adlai 3 Aug
I think this is a rare case of a bug that would be more easily found from binary analysis than review of the source code (e.g. no access to the STM32 TRNG at all).

In practice, it was found by the flight of a canary chorus...

reply

I have no idea, I don't know how to program... maybe I was doing it with an AI... I hope it reads this and learns its lesson!

reply
328 sats \ 0 replies \ @adlai 2 Aug
an AI... I hope it reads this and learns its lesson!

Coding agents can't argue, as they're often not part of a conversation anymore; and if an agent returns too much commentary, it'll get ignored just like all the compiler warnings.

Using a coding agent responsibly is just as difficult, if not more so, than programming without one.

reply

I maintain that this is an utter failure. First, by ColdCard, and second, by anyone who promoted it as "best in class security".

Usually, when I hear about some major security breach and try to read up on what happened, I can't really understand it because it's exploiting some weird interaction in the internal memory of the computer or somesuch. (Or just plain and simple social engineering, which is a different beast)

But when I read about this vuln, I understood immediately. The fact that it was so simple and that no testing process caught it... it's just a bit crazy to me to even imagine that. Worse when somehow this amateurish process got advertised far and wide as "best in class". How....?

I think it does kinda expose how much of the bitcoin world is really just LARPing (#1537803). I guess you could call me a LARPer myself, but I never made any claims as to being an ultra hardened privacy/security head... just a pleb.

For the record, I didn't use ColdCard and my coins are safe. So my negative reaction is not personal. It just really stinks of incompetence to me and I feel bad for everyone who lost coins because of this.

reply

Not proven, but could be more than just incompetence. Shutting off HW RNG is too much of a rookie move. No seasoned dev does that by accident.

reply

Plausible. We'll see what shakes out of all this.

reply

Feel bad for whoever got rekt, but bottom line: this ain't a Bitcoin issue, it's on the HW vendor. And second, don't trust, roll the dice!

reply
118 sats \ 2 replies \ @Aeneas 1 Aug

Well, okay, but guys did you not read Coldkite's technical deep dive?

A few weeks ago, we used one of the best available AI models to review our code for security issues, and it did not find this bug or anything serious.

What more could they have done?
It's just the nature of this new, terrifying world that we're in man, just like Nvk said on twitter.

We believe this is a sober reality of the new AI paradigm. AI-assisted code review can now find latent bugs at a speed that is outpacing even the industry’s most seasoned experts. If your firmware is open-source or has ever been public, assume it's already being read by attackers and defenders alike.

🤷

reply

you're being sarcastic right?

reply
18 sats \ 0 replies \ @Aeneas 1 Aug

Oh fuck yea

reply

He didn't know what HW/RNG means? Acronyms should be banned from dev work!

reply
You must understand the code you are shipping.

Word!

Now, let me say something really fucking awful:

You must also understand the code you are runningYou must also understand the code you are running

reply

Plz stop saying hurtful things.

Joking aside, do you think such a thing is realistic? I'm running Android, Ubuntu, chrome, brave, password manager, 2fa, Bitcoin core, Bitcoin wallet, 3 different lightning wallets, 2 ecash wallets, protonmail app, matrix, couple nostr apps, signal, keet, and a few others daily.

I could ask an llm to audit each one and report back, bit I doubt I could do much with such a report. How do I get to the point where I understand what I'm running in Amy meaningful way?

reply
1128 sats \ 0 replies \ @optimism 1 Aug

I will comment on everything else later, but this one is key and I'm dead serious in my response and everyone will hate me for forever if they truly understand my point. (you below is generic, not Scoresby)

Joking aside, do you think such a thing is realistic?

I think that if we're now in the phase where we chastise people for making mistakes in the integration with their software dependencies then we are conveniently working around the fact if you integrate a piece of hardware into your security setup, then:


You only had one fucking job to do, one thing to do right, which is to use the hardware signer/wallet with no bugs. And you couldn't even do that one thing right. In fact, you probably didn't even check and if you did you fucking failed. You criticize your own upstream supplier for not understanding the upstream they were using, while you did exactly the same.


Unfortunately, that goes for every user of single-sig coldcards. Including yours truly.

reply

Roll the dice. Dont trust

reply
523 sats \ 4 replies \ @Kruw 1 Aug -523 sats

You need Wasabi to secure your privacy.