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

261–270 of 276 posts

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

#261

Post by nameless912 »

jasonvorhe wrote: post #245

Reading stuff like "heads gonna roll" in the comments is kind of shocking to me. Have you never screwed up in your job? How long have they shipped a secure Linux to a couple of hundred different devices simultaneously, while improving security, fixing critical bugs soon and delivering a pretty great user experience since they first launched Chrome OS? Sure, were soft bricks before, critical fixes introduced other bug…

Yup. My second or third month at my first job, I accidentally knocked out internet access for thousands of people for a couple of hours because I deleted something in production rather than in dev. It was terrifying and I was sure I would be fired.

Rather than being canned, of course, the team managing the tool that allowed me to delete the resource learned the following:

1) people should not be able to delete things without confirmation in production

2) it should be very obvious which environment you're in

3) normal humans shouldn't have access to production except for operations folks.

That's the value of blameless post mortems.

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

#262

Post by bobbyi_settv »

JonathonW wrote: post #128
saagarjha wrote: post #118

Earlier quoted context omitted.

Short circuiting boolean operators are very useful, and not something I would consider a code smell. They let constructs like this work, which are very common in most codebases: if (foo && foo->bar) { // whatever } An operator that did not short circuit would have undefined behavior because it might dereference a NULL pointer.

If you're going for "each line of code should do exactly one thing", you'd probably prefer that as if(foo) { if(foo->bar) { //whatever } } separating out the null check and the actual conditional. More lines of code and more nesting, yes, but, if you're trying to strictly adhere to a one thing/one line principle, you probably don't care. Short-circuit 'or' is a little harder to avoid (if you specifically want the sho…

That technique doesn't work (as well) when you want a single "else" block that is used when either of the conditions is false, which I think is pretty common for this sort of conditional

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

#263

Post by anyfoo »

GoblinSlayer wrote: post #192
anyfoo wrote: post #86

Earlier quoted context omitted.

Why would you even need non-shortcircuiting behavior in boolean expressions? Because either side of the operator has side effects that you want to happen unconditionally? Please just write it out as an extra statement then instead of hammering it into place with an implicit coercion to an integer type only so you can abuse bitwise AND and OR only to force that side effect to... you can see why this is probably not th…

When you check for error conditions shortcircuiting is unlikely and thus useless. if(error1|error2)abort();

But it semantically resolves to the exact same, and so the compiler will likely treat it the same and optimize for whatever makes most sense. This may both make a written "a|b" short-circuiting, and a written "a||b" non-short-circuiting, as long as the result stays the same (which for simple bool a,b it always does).

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

#264

Post by anyfoo »

thaumasiotes wrote: post #175
anyfoo wrote: post #86

Earlier quoted context omitted.

Why would you even need non-shortcircuiting behavior in boolean expressions? Because either side of the operator has side effects that you want to happen unconditionally? Please just write it out as an extra statement then instead of hammering it into place with an implicit coercion to an integer type only so you can abuse bitwise AND and OR only to force that side effect to... you can see why this is probably not th…

> Why would you even need non-shortcircuiting behavior in boolean expressions? Because either side of the operator has side effects that you want to happen unconditionally? Interestingly, the example provided by slavik81 is able to tolerate non-shortcircuiting behavior specifically because no side effects are involved. It's more of a case of "we don't care about the result of this work, but we have to do it anyway, b…

It's usually wrong to assume that expressions that ultimately have the same effect (such as a || b and a | b when both a and b are simple booleans, so no side effects) will make the compiler generate different code depending on which variant to use.

The compiler knows that both variants resolve to the same result with no side effects, so it should try to generate whatever code is more efficient. Depending on several factors, that may be machine code that effectively exhibits short circuit behavior or not, for either expression (so even a|b might end up having a short circuit on the lowest level).

This would be different if you, e.g., sprinkle some volatile keywords (which effectively introduces side effects), but then you'd probably still want to take more care and split the expressions up, unless you really don't care about the order of your forced memory accesses (i.e. if I recall correctly, there is no guarantee that a|b will access a first, then b). And that's not even going into barriers.

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

#265

Post by asddubs »

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

you can even use it for rvalues!

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

#266

Post by dataflow »

khazhoux wrote: post #139
dataflow wrote: post #27

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

I don't think that's right. It seems far-fetched that the compiler would know that accessing key_data->label() implies that k_d->has_value() == true I believe the explanation is: A && B : compiler will evaluate B only if A==true A & B : compiler will evaluate both A (safe) and B (unsafe), then perform bitwise operation

No, it's not far-fetched, this is a pretty well-known optimization.

https://blog.regehr.org/archives/970

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

#267

Post by thaumasiotes »

GoblinSlayer wrote: post #221
thaumasiotes wrote: post #173

Earlier quoted context omitted.

That's not true. Compare `( a() || b() || c() ) ? yes() : no()` to `( a() | b() | c() ) ? yes() : no()`. In the second case, there are two paths through the code: a() b() c() yes() a() b() c() no() But in the first case, there are more paths: a() /* true */ yes() a() /* false */ b() /* true */ yes() a() /* false */ b() /* false */ c() /* true */ yes() a() /* false */ b() /* false */ c() /* false */ no() That is becau…

The code is correct if you hit yes() and no() correctly depending on three conditions. There are 8 possibilities in both cases and thus you need 8 tests in both cases. Shortcircuiting is optimization, it doesn't reduce complexity of the logic.

That is a concern, but it's not one related to how many branches exist in the code.

"How did we get here?" and "Are we in the right place?" are distinct questions.

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

#268

Post by thaumasiotes »

snovv_crash wrote: post #242
thaumasiotes wrote: post #173

Earlier quoted context omitted.

That's not true. Compare `( a() || b() || c() ) ? yes() : no()` to `( a() | b() | c() ) ? yes() : no()`. In the second case, there are two paths through the code: a() b() c() yes() a() b() c() no() But in the first case, there are more paths: a() /* true */ yes() a() /* false */ b() /* true */ yes() a() /* false */ b() /* false */ c() /* true */ yes() a() /* false */ b() /* false */ c() /* false */ no() That is becau…

So, it tests that you're running yes() and no(). But it doesn't test that your choice to use 'a()|b()|c()' for your condition was correct, instead of, for example, 'a()&b()&c()' or even '(a()&b())|c()' because you didn't test all the combinations. Using bitwise instead of logical operations hides this in your branch coverage report.

You are not attempting to have branch coverage; you are attempting to cover the space of possible inputs.

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

#269

Post by cmeacham98 »

0x0 wrote: post #230
SahAssar wrote: post #22

Just reading the title it sounded very much like "Single Point of Failure: The (Fictional) Day Google Forgot To Check Passwords": https://www.youtube.com/watch?v=y4GB_NDU43Q

This actually happened for real with Dropbox about 10 years ago: https://techcrunch.com/2011/06/20/dropbox-security-bug-made-...

That exact news article appears in the video.

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

#270

Post by perl4ever »

encryptluks2 wrote: post #176
perl4ever wrote: post #119

Earlier quoted context omitted.

>They don't get sued, they do a recall and fix it Are you kidding? Major (car, drug, you name it) companies have hundreds, if not thousands of lawsuits going at any given moment. Public companies will sometimes list major lawsuits in their annual report, but most of them don't get mentioned because the amount at stake is trivial for them.

You have to remember, pretty much anyone can sue anyone... doesn't mean that they'll be successful.

I don't have statistics on that, but I find it implausible that there would be hundreds or thousands of lawsuits against a typical corporation if say, only a couple % won or got a settlement.
Post reply on HN