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

21–30 of 276 posts

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

#21

Post by CobrastanJorji »

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]…

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

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

#23

Post by tmp_anon_22 »

5350-uiop-1130 wrote: post #16

Couldn't this be caused by a bad merge? Not sure how Google manages their merging and release process but there are conceivably several stages where a bad merge can occur and go unnoticed - possibly at a stage close to release and after testing.

At an established company merging is an incredibly incredibly locked down process. There are usually multiple barriers of automation and people, entire release teams who all have to be stoned at the wheel to miss anything odd getting through.

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

#24

Post by nicoburns »

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 offence

I'd probably agree with that. Firing should be for extreme negligence, or sustained underperformance.

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

#25

Post by kevinmgranger »

CobrastanJorji wrote: post #21
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]…

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

If I had to guess, the second expression depends on the first being true. So if `has_value()` is false, perhaps `key_data_->label()` is undefined behavior.

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

#26

Post by Trollmann »

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]…

You can use `and` and its siblings instead of && and similar in C++11. Most people here, and in other boards, will try to convince you that it hurts readability because 'that‘s how we always did it' (read I‘m used to it and don‘t like change).

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

#27

Post by dataflow »

CobrastanJorji wrote: post #21
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]…

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

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

#28

Post by dylan604 »

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]…

That so sounds like an error I would make and not some superDev working at the Googs making around 4x the amount of money I make.

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

#29

Post by nogridbag »

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…

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.

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

#30

Post by fouric »

Trollmann wrote: post #26
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]…

You can use `and` and its siblings instead of && and similar in C++11. Most people here, and in other boards, will try to convince you that it hurts readability because 'that‘s how we always did it' (read I‘m used to it and don‘t like change).

I am also of the opinion that `and` is more readable than `&&` (and isn't as easy to typo in a catastrophic way) - although my main point was about the weaker type system.
Post reply on HN