Hacker News Board Hacker News, browsed like a classic message board
Live data from Hacker News

When random.bytes() runs but doesn't work

insider.btcpp.dev

Post reply on HN
Print view View original

51–60 of 63 posts

Re: When random.bytes() runs but doesn't work

#51

Post by inigyou »

nullc wrote: post #50
dale_glass wrote: post #48

Earlier quoted context omitted.

The my_random_bytes implementation doesn't look good at all at first glance. Let's see: 1. I believe it errors out if the HWRNG returns the same value twice. That's actually a thing that can legitimately happen. "0" is also a legitimate output. 2. "here" is a terrible name for a length 3. It does a memcpy of a minimum of 4 bytes to the destination, even if count is lower. It'll also overflow longer buffers with a len…

I have absolutely no interest in defending this code-- and already considered it untrusthworthy before any of this. But in the interest of accuracy: > It does a memcpy of a minimum of 4 bytes This is a common misreading of MIN(). MIN(4,x) is a number that is a MAXIMUM of 4, not a minimum. Count is the number of bytes remaining in the buffer. The input to the copy is a 4-byte word. min(4,count) will produce a number 0…

There have been cases where a small statistical bias like avoiding duplicate bytes can be amplified to a full break. I don't think this problem is applicable to a bitcoin private key though.

Re: When random.bytes() runs but doesn't work

#52

Post by nullc »

inigyou wrote: post #51
nullc wrote: post #50

Earlier quoted context omitted.

I have absolutely no interest in defending this code-- and already considered it untrusthworthy before any of this. But in the interest of accuracy: > It does a memcpy of a minimum of 4 bytes This is a common misreading of MIN(). MIN(4,x) is a number that is a MAXIMUM of 4, not a minimum. Count is the number of bytes remaining in the buffer. The input to the copy is a 4-byte word. min(4,count) will produce a number 0…

There have been cases where a small statistical bias like avoiding duplicate bytes can be amplified to a full break. I don't think this problem is applicable to a bitcoin private key though.

Right, that's particularly a concern for DSA nonces. Coldcard uses RFC 6979 however, and duplicates at the 32-bit word level is a different matter than at the byte level.

Re: When random.bytes() runs but doesn't work

#53

Post by nullc »

jki275 wrote: post #38
eru wrote: post #4

Earlier quoted context omitted.

Also: writing this stuff in C just begs for bugs. LLM agents are cheap and good enough that you can write in eg Lean or whatever. Or at least write it in Rust.

C isn't the problem here. one might argue that the combination of micro python and C and the person writing it having no understanding of how either one works is the problem. Also running python on a microcontroller to do cryptography is fucking insane.

> Also running python on a microcontroller to do cryptography is fucking insane.

I feel like I should confess to snubbing this (and some other) hardware wallet projects for the fact it used micropython. I think it's hard to draw the line between systems programmer snobbery and good advice... and I feel a little like a cop guilty of stop-and-frisk on the basis of skin color.

Because in general the ideas necessary to produce reliable software aren't well understood or agreed on there is a risk of letting style preferences which are only correlated with good engineering but aren't causative of good engineering get mistaken-- and this can cause errors in both directions, both mistaking stuff as good because it uses the "right" tools, or mistaking something as bad because it doesn't.

Re: When random.bytes() runs but doesn't work

#54

Post by nullc »

scott_w wrote: post #42
thin_carapace wrote: post #6

Earlier quoted context omitted.

I get the feeling that the bugs found in this instance can be more directly attributed to the author having zero experience operating a microcontroller

Honestly, I'm going to be a bit harsher: I think they're either: 1. A novice who doesn't know how to debug issues. 2. Totally incompetent and copying code from Stack Overflow. I don't think it's specific to microcontrollers, either, the error from the C compiler is something that a competent programmer would be able to interpret, or at least see it as a signal to get someone more experienced in the domain to learn ab…

> the error from the C compiler is something that a competent programmer would be able to interpret,

contrary to the post, this was almost certainly not an issue of compilers throwing errors.

Reduced familarity with C could have played a role, but given the number of C experts that looked at this knowing there was an error and still misidentified the cause I don't think we need to reach for that powerful an explanation.

Confusion of definedness vs value check would make for a fine underhanded C entry. The flaw was not particularly clear from the source... and most common QA procedures could not distinguish a PRNG from TRNG once the error happened.

Re: When random.bytes() runs but doesn't work

#55

Post by nullc »

abecedarius wrote: post #36
nonfamous wrote: post #17

From the Twitter advisory on the issue being referenced here [1]: >>> To every other developer: 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. Kinda turns the “many…

If your source is not open that's barely a speedbump, given a binary. At least that's my impression of the current state.

This flaw would probably be easier to find from the binary: the source itself was misleading, but if you analyized the binary you'd find that there was no access to the hwrng at all (assuming the dead code got eliminated by the compiler) or that the only function that accessed it was never called.

One lesson out of this is that now that AI has made a certain grade of review cheap is that it would be useful to perform security review both against the source code and against the resulting binary.

Re: When random.bytes() runs but doesn't work

#56

Post by coldbrewed »

koolba wrote: post #28

> A good goal as a developer is a high commit message to change ratio. The more lines of code that you change, the more comments explaining why you’re changing the code. More message and less code changes per commit is generally a good idea. No! Blanket statements like this is how you end up with 40 pages of slop AI comments in PRs that nobody reads. Comments should be terse and meaningful. They should document surpr…

We shouldn't tolerate slop in any form, especially not in commit comments. What we should care about is communicating design decisions, intent, and especially we should explain concerns that can't be intuited from reading the code. A longer, more detailed comment that explains some significant detail can be especially valuable but it should earn the weight that it carries.

I think we're all in agreement that a one word "runs" commit and a 6 paragraph slop commit both fail to communicate effectively.

Re: When random.bytes() runs but doesn't work

#57

Post by nullc »

smithcoin wrote: post #16
nullc wrote: post #9

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…

Have you found a write up you’d recommend on how the threat actors actually exploited the vulnerabilities?

Hopefully people are being a little circumspect right now with explotation instructions because there are still vulnerable coins out there (particularly from mk4+ wallets, as well ones with passwords that are not strong enough) which have not been taken or rescued yet and may still be rescued by owners.

Re: When random.bytes() runs but doesn't work

#58

Post by killerstorm »

nullc wrote: post #9

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…

Well, if the developer read libngu's random.c he would have noticed that it uses rng_get() and that it's asking for MICROPY_HW_ENABLE_RNG (albeit the failsafe is defective, reading it should have at least triggered "wait, what?").

So I'd say the root cause that the developer didn't check what random.bytes() does under the hood or how libngu expects random to be generated.

Re: When random.bytes() runs but doesn't work

#59

Post by scott_w »

nullc wrote: post #54
scott_w wrote: post #42

Earlier quoted context omitted.

Honestly, I'm going to be a bit harsher: I think they're either: 1. A novice who doesn't know how to debug issues. 2. Totally incompetent and copying code from Stack Overflow. I don't think it's specific to microcontrollers, either, the error from the C compiler is something that a competent programmer would be able to interpret, or at least see it as a signal to get someone more experienced in the domain to learn ab…

> the error from the C compiler is something that a competent programmer would be able to interpret, contrary to the post, this was almost certainly not an issue of compilers throwing errors. Reduced familarity with C could have played a role, but given the number of C experts that looked at this knowing there was an error and still misidentified the cause I don't think we need to reach for that powerful an explanati…

Fair point: I was using the information in the article to form my opinion. I still think that, if the developer in question ignored a compiler error as described, that’s a serious competence issue. It’s harder to spot after the fact because any reviewers wouldn’t have had a chance to see the compiler output.

The article makes the fair point of the size of the change and the complete lack of information in the commit message, which should have set off alarm bells…

Re: When random.bytes() runs but doesn't work

#60

Post by eru »

scott_w wrote: post #42
thin_carapace wrote: post #6

Earlier quoted context omitted.

I get the feeling that the bugs found in this instance can be more directly attributed to the author having zero experience operating a microcontroller

Honestly, I'm going to be a bit harsher: I think they're either: 1. A novice who doesn't know how to debug issues. 2. Totally incompetent and copying code from Stack Overflow. I don't think it's specific to microcontrollers, either, the error from the C compiler is something that a competent programmer would be able to interpret, or at least see it as a signal to get someone more experienced in the domain to learn ab…

Of course, being a total novice and claiming to produce a super secure gadget are not to things you want to go together.
Post reply on HN