Hacker News Reader

Comment by bombcar | Hacker News Reader

Apparently the coding mistake was identified and deemed "impossible to exploit": https://certificate.quantstamp.com/full/nomad

> QSP-19 Proving With An Empty Leaf

> Recommendation: Validate that the input of the function is not empty

> The Nomad team responded that "We consider it to be effectively impossible to find the preimage of the empty leaf".

> We believe the Nomad team has misunderstood the issue. It is not related to finding the pre-image of the empty bytes. Instead, it is about being able to prove that empty bytes are included in the tree (empty bytes are the default nodes of a sparse Merkle tree). Therefore, anyone can call the function with an empty leaf and update the status to be proven.

View on HN

It seems like it was (at least arguably) impossible to exploit until they introduced a second issue:

>It turns out that during a routine upgrade, the Nomad team initialized the trusted root to be 0x00. To be clear, using zero values as initialization values is a common practice. Unfortunately, in this case it had a tiny side effect of auto-proving every message

EDIT:

Reading and noodling I'm 99% sure these are separate issues. The vulnerability talks about passing in an empty leaf to the prove function. But that's not what the exploit is.

The exploit is using an unproven message. So they are passing in an actual leaf to prove. The problem is that unproven messages have 0x00 as root and some jabroni set 0x00 as the trusted root. So every message was treated as proven by default when it should be the opposite.

> routine upgrade

yes, a routine upgrade. that's what it was..... (→_→)

The cofounder was involved in some drama with a CELO bridge not too long ago

https://imgur.com/a/gV6s1Gw

... that he also founded and planned to spin out with Celo, yes. James is the inventor of Optics, the cross-chain communication technique Nomad uses.

Very unhappy with how Celo handled the situation, as was much of the technical crypto community at the time.

Oh interesting! Seems like some heavy gaslighting by Celo then

Anyone who's studied NTSB reports knows this is almost always how they go; a single failure that is no problem becomes one because of some other issue that doesn't normally happen, or couldn't happen because normally a third thing is always done ...

There are several very interesting books about this, yes. Air disasters are fascinating to read about.

A series of very good, detailed article about air disasters over on Reddit: https://www.reddit.com/r/AdmiralCloudberg/comments/e6n80m/pl...

Same with tons of security incidents in general. It's the "swiss cheese model" (https://en.wikipedia.org/wiki/Swiss_cheese_model).

And when it becomes organisationally normalised that one layer of safety isn't working and the remaining ones are relied upon, then that's more like the subject of Normal Accidents [1]

[1]: https://en.wikipedia.org/wiki/Normal_Accidents

Stable systems don't break because something went wrong, they break because everything went wrong.

I'm not completely sure the mechanics of this exploit, but I've recently adopted the personal guideline of "all enums and integer IDs start at 1", and 0 is simply an invalid value.

Not a hard and fast rule, and not something that will catch tons of problems, but now and again it does help me catch an uninitialized value.

Or use bigger numbers, so enums that get mixed up somehow won’t work.

> "all enums and integer IDs start at 1"

then whoever takes over maintenance from you and wants to make improvements says "hey, we can add this new v2 feature through the 0x0 which is available for future enhancements"

Unlikely. The way it works is you define your enums like this:

  enum class Color {
    Invalid, // Not a valid value.
    Red,
    Orange,
    Yellow,
    Green,
  };

I think if someone's crazy enough to reclaim a 0 value in an enum or integer ID, then they're probably going to do a lot of damage to the code no matter what you do.

Yeah, pretty much every integer enum I write has `UNKNOWN` or `INVALID` as the zero value. It adds either a `case` or a `default` to every switch and a clause or a `_` to every `match`, but it saves my ass even in non-security-sensitive settings.

That wouldn’t work in situations like this unfortunately. In the ethereum ecosystem the 0x0 address has a special meaning because it is used as the origin of things on chain when they are newly created (they are transferred from 0x0) and often also the destination of things when you want to destroy them (you “burn” something by transferring it to 0x0).

Fun part is as far as I understand this is just a convention and there is nothing intrinsic to 0x0 that makes it different from any other destination address. If someone was to find the private key for which 0x0 is the public half they would have access to a vast amount of stuff that has been transferred there over time. Finding this key is computationally unfeasible however.

Why wouldn’t it work here? Couldn’t you just as easily use address 1 or all ones for the same purpose?

I don't think this is the same issue as the exploit.

The real issue was half-caught in a review on a pull request however. https://github.com/nomad-xyz/monorepo/pull/289/files

If this legacy enum value had been handled later in the code, there would not have been a vulnerability.

(This isn't to say that the developers were bad. The person who wrote the code was extremely knowledgeable. It's just really hard to be perfect every time. )

No, they are bad.

They are bad because they are not competent to write the decent code required by their profession and job environment.

In normal software writing trade, such engineers are called low performers and routinely managed out of any organization.

Sure, the mistake is not unusual from the perspective of general software engineering. But let's not forget what software they are working on.

I am totally fine with a bartender dropping a glass... I'll put a surgeon on trial if he cannot make his hands steady during a heart surgery...

Even excellent devs make security mistakes. That's why good teams build out processes like auditing, fuzzing, heuristics checkers, and require internal peer review.

This "you must be perfect" mentality is detrimental to building a security culture, IMO — no one is perfect, and the most excellent dev will slip up. Seeing the people involved in that commit, I believe that's what happened here.

Hoping we learn more in the post-mortem, and they revise their practices to catch this mistake in the future.

Why are the bartender and surgeon held to different standards?

They aren't held to different standards. They are doing different work.

Lmao, people make mistake dude. Because they're human. What is seen here is a failure of multiple people, an organisation.

I remember AWS S3 went down in 2017 or so and the key point I took away from their article about it was that they didn't blame the junior dev that caused it, because it shouldn't have been able to happen in the first place.

Ah here we are: https://aws.amazon.com/message/41926/ "We are making several changes as a result of this operational event..." basically boiling down to "the employee in question is not at fault because our tools should not have let him do that".

That's common misinformation. The issue discussed in the review has no connection to what happened, and the narrative that it is the same issue, but it became exploitable after the update is incorrect too. It affects different part of smart-contract logic. Yes, it's pretty close at a glance—because you have zero-by-default problem somewhere close to Merkle tree.

Quantstamp said it was exploitable. Nomad falsely claimed it wasn’t.

They're two distinct issues in two distinct contracts, AFAICT.

Reply on news.ycombinator.com