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

Google broke a conditional statement that verifies passwords on Chrome OS

arstechnica.com

Post reply on HN
Print view View original

31–40 of 276 posts

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

#31

Post by fouric »

dataflow wrote: post #27
CobrastanJorji wrote: post #21

Earlier quoted context omitted.

Forgive my poor C++, but has_value() sounds like a bool, and empty() sounds like a bool. Why wouldn't "bool1 & bool2" work correctly?

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.

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

#32

Post by sneak »

nogridbag wrote: post #29
sneak wrote: 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.

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, developed and used by Google, the organization under discussion) have designed their syntax now to completely avoid this type of error even being possible.

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

#33

Post by secondcoming »

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

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

#34

Post by admax88q »

sneak wrote: post #20
mdoms wrote: post #18

Earlier quoted context omitted.

> Heads should probably roll here, I disagree. It's obviously a major balls up by multiple people, but at the end of the day this is an organizational failure. You don't fire people for this, you learn from it and fix the organizational holes that caused it.

Someone is responsible for the organizational units and their processes. By that logic, almost no individual failure is a firing offense. Many people in the organization are responsible for ensuring that the processes themselves overlap in ways that exclude the possibility of failures like this. I'm talking about the meta-failure in management's oversight of the processes. There is belt person, and there is suspender…

The way you fix organizational/process failures is not by firing the person in charge of them every time they fail to catch something.

How many breaking bugs have _NOT_ shipped due to the current processes. Your approach suggests that there is some perfect process manager out there who will let no bugs like this ship (whilst not dropping any other business priorities mind you) that they just haven't hired yet into this position.

This isn't as big of a bug as people on HN want it to be. A few months from few people will remember, and even fewer will care.

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

#35

Post by idiotsecant »

nicoburns wrote: post #24
sneak wrote: post #20

Earlier quoted context omitted.

Someone is responsible for the organizational units and their processes. By that logic, almost no individual failure is a firing offense. Many people in the organization are responsible for ensuring that the processes themselves overlap in ways that exclude the possibility of failures like this. I'm talking about the meta-failure in management's oversight of the processes. There is belt person, and there is suspender…

> 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?

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

#36

Post by yupper32 »

sneak wrote: post #20
mdoms wrote: post #18

Earlier quoted context omitted.

> Heads should probably roll here, I disagree. It's obviously a major balls up by multiple people, but at the end of the day this is an organizational failure. You don't fire people for this, you learn from it and fix the organizational holes that caused it.

Someone is responsible for the organizational units and their processes. By that logic, almost no individual failure is a firing offense. Many people in the organization are responsible for ensuring that the processes themselves overlap in ways that exclude the possibility of failures like this. I'm talking about the meta-failure in management's oversight of the processes. There is belt person, and there is suspender…

> 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 move on with their lives.

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

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

#37

Post by [deleted] »

fouric wrote: 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]…

[deleted]

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

#38

Post by sneak »

yupper32 wrote: post #36
sneak wrote: post #20

Earlier quoted context omitted.

Someone is responsible for the organizational units and their processes. By that logic, almost no individual failure is a firing offense. Many people in the organization are responsible for ensuring that the processes themselves overlap in ways that exclude the possibility of failures like this. I'm talking about the meta-failure in management's oversight of the processes. There is belt person, and there is suspender…

> 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.

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

#39

Post by taeric »

secondcoming wrote: post #33

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 short circuiting was preventing undefined behavior on the second expression.

That is, yes, it is the same with booleans. Now try with expressions.

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

#40

Post by fierro »

sneak wrote: post #10
Traster wrote: post #8

I bet the post-mortem for this is going to be fun. How this wasn't covered by multiple unit tests, a code review and a roll out strategy is.... impressive.

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…

glances at screen LGTM
Post reply on HN