Live data from Hacker News

Google broke a conditional statement that verifies passwords on Chrome OS

arstechnica.com

41–50 of 276 posts

Re: Google broke a conditional statement that verifies passwords on Chrome OS

#41

Earlier quoted context omitted.

> By that logic, almost no individual failure is a firing offence I'd probably agree with that. Firing should be for extreme negligence, or sustained underperformance.

>extreme negligence If this doesn't qualify, what does?

I would say this is negligence, but perhaps not extremely so.

Re: Google broke a conditional statement that verifies passwords on Chrome OS

#42
post #10

Earlier quoted context omitted.

Bitwise & versus logical && is a classic, right up there with an assignment in a comparison (when an equality check is intended), = for ==. That this was missed is pretty surprising, given that it's Google and the stakes involved in the encryption/key management code in a secure platform device. I wonder if we'll even get a postmortem, as this simply cannot happen unless several someones all Seriously Fucked Up simul…

I've seen the = instead of == in the wild with disastrous results: the assignment was on an ORM backed object, e.g. // accidentally mutates all names to Sam and persists to the DB myList.filter { myDomainObj.name = 'Sam' } But it makes me wonder why our programming languages would use characters which can often lead to this type of error.

I'm a fan of programming languages using := for assignment for this reason

Re: Google broke a conditional statement that verifies passwords on Chrome OS

#43
post #31

Earlier quoted context omitted.

Dereferencing via key_label_-> has undefined behavior when !key_label_.has_value(), so the compiler treats that case the same as if key_label_.has_value(). Or to put it another way, the compiler reasons that key_label_ must have had a value (since you dereferenced it), and thus optimizes out the check for `has_value()` (since clearly it was unnecessary).

Ahhh, so it was a problem with lazy evaluation (and the lack thereof when using &), and not type coercion, like I initially thought.

Yeah it has nothing to do with type coercion. I'm baffled a linter doesn't catch this though. & for bools should just be prohibited altogether. If people really want & for bool, they can just cast to an integer type and use & with that (which generates its own warnings appropriately).

Re: Google broke a conditional statement that verifies passwords on Chrome OS

#44

Edit: Someone has explained the issue below I don't understand how that broke anything? The C++ codegen for && and & is the same: bool good(bool a, bool b) { return a && b; } good(bool, bool): mov eax, edi and eax, esi ret bool bad(bool a, bool b) { return a & b; } bad(bool, bool): mov eax, edi and eax, esi ret [0] https://godbolt.org/z/84MMqTehs

The code is:

    key_data.has_value() && !key_data_->label().empty()
key_data is probably something like an std::optional, so short-circuit evaluation guards the dereferencing of the optional. If you write this with a binary and, no short-circuit evaluation can happen, so undefined behavior ensues.

I wanna point out that std::optional, in the typical C++ stance, has a "I know what I'm doing" API and a "Humans make mistakes API": optional->... is UB if the optional is empty, optional.value()... raises an exception. Arguably, in code like this you probably only want to use the latter kind of API. (The pattern of "operator can UB, equivalent method asserts pre-condition" is very common in the C++ standard library)

Re: Google broke a conditional statement that verifies passwords on Chrome OS

#45

Edit: Someone has explained the issue below I don't understand how that broke anything? The C++ codegen for && and & is the same: bool good(bool a, bool b) { return a && b; } good(bool, bool): mov eax, edi and eax, esi ret bool bad(bool a, bool b) { return a & b; } bad(bool, bool): mov eax, edi and eax, esi ret [0] https://godbolt.org/z/84MMqTehs

The difference is your example uses bool values, the code in question uses expressions that (should) evaluate to bool. && is short-circuited if a is false, such that b is not evaluated.

Re: Google broke a conditional statement that verifies passwords on Chrome OS

#46
post #32

Earlier quoted context omitted.

I've seen the = instead of == in the wild with disastrous results: the assignment was on an ORM backed object, e.g. // accidentally mutates all names to Sam and persists to the DB myList.filter { myDomainObj.name = 'Sam' } But it makes me wonder why our programming languages would use characters which can often lead to this type of error.

If you consider a linter part of a programming language, most, when properly configured, do not permit this. It's just the syntax that does; professionals do not rely on syntax validity alone but employ linting, code review, static analysis, and integrations tests on top. The gradient of professionalism in our industry is very wide, and the slope is quite shallow. To your point, however, many languages (such as Go, d…

> type of error even being possible

I mean its still possible to forget the ":" character and its still possible to mentally scan a PR and see "=" and miss that it should have been a "==".

Re: Google broke a conditional statement that verifies passwords on Chrome OS

#47

Edit: Someone has explained the issue below I don't understand how that broke anything? The C++ codegen for && and & is the same: bool good(bool a, bool b) { return a && b; } good(bool, bool): mov eax, edi and eax, esi ret bool bad(bool a, bool b) { return a & b; } bad(bool, bool): mov eax, edi and eax, esi ret [0] https://godbolt.org/z/84MMqTehs

[deleted]

Re: Google broke a conditional statement that verifies passwords on Chrome OS

#48
post #17

From the article: > The line should read "if (key_data_.has_value() && !key_data_->label().empty()) {" but instead of "&&"—the C++ version of the "AND" operator—the bad update used a single ampersand, breaking the second half of the conditional statement. Someone correct me if I'm wrong, but this seems like it wouldn't be possible in a language that didn't conflate boolean values with bitvectors. Edit: I was wrong[1]…

This is one of those times when, for all its verbosity, Ada got it right. For boolean conjunction you must choose between writing "A and B", or "A and then B". The "and then" version is short circuiting, while the bare "and" version is not.

Re: Google broke a conditional statement that verifies passwords on Chrome OS

#49
post #38

Earlier quoted context omitted.

> By that logic, almost no individual failure is a firing offense. That's pretty close to how it should be. That's the core premise of the no-blame post-mortem culture that's great about some companies. As long as you go through the port-mortem process, which includes doing your best to prevent this from happening again (which may include changing organizational processes, testing strategies, etc), then everyone can…

This isn't an engineering failure. Everyone writes bugs.

Engineering process is still engineering in my book, but if it helps:

Firing happens when you have a history of performing below your level. Never for a single process failure.

Re: Google broke a conditional statement that verifies passwords on Chrome OS

#50
post #10

Earlier quoted context omitted.

Bitwise & versus logical && is a classic, right up there with an assignment in a comparison (when an equality check is intended), = for ==. That this was missed is pretty surprising, given that it's Google and the stakes involved in the encryption/key management code in a secure platform device. I wonder if we'll even get a postmortem, as this simply cannot happen unless several someones all Seriously Fucked Up simul…

I've seen the = instead of == in the wild with disastrous results: the assignment was on an ORM backed object, e.g. // accidentally mutates all names to Sam and persists to the DB myList.filter { myDomainObj.name = 'Sam' } But it makes me wonder why our programming languages would use characters which can often lead to this type of error.

The problem is not even with "easy to mix up operators" for this, it's about languages without a strong and static enough type system. While the particular problem Google hit is a bit more subtle (involving undefined behavior), in the majority of cases a strongly typed language would not allow expressions that accidentally mix up = and ==, as they often resolve to different types.

In C "if (foo = bar)" and "if (foo == bar)" are both legal, because pretty much any type can be automatically coerced into a boolean. In many modern languages that is not the case. (Of course if you have assignments as expressions you are still screwed if foo and bar are both boolean to begin with, so some languages got away with that, too. And for fairness I should say that C at the very least warns if you don't explicitly put parentheses around the assignment, nowadays.)

Post reply on HN