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

Reverse Engineering MacOS High Sierra Supplemental Update

cocoaengineering.com

Post reply on HN
Print view View original

11–20 of 135 posts

Re: Reverse Engineering MacOS High Sierra Supplemental Update

#11

Post by atas »

blowski wrote: post #5
atas wrote: post #2

Simple technique for preventing bugs like this: don't copy-paste code. If you find yourself copy-pasting code think twice why you even have to do it (DRY principle) and be aware of the potential consequences. Even if some code has to be duplicate, I am forcing myself to just write it from scratch, exactly for this reason: do you really know that you have updated all your data? And yeah, unit tests would help in spott…

Just to play devil's advocate: sometimes DRY code is harder to understand, which makes it harder to see bugs. Whenever you introduce an abstraction to make your code DRYer, you have to remember the law of leaky abstractions. DRY code is a good principle to follow, but not an absolute law.

Yes, and that's why you have to be extra-careful with copy-pasting :-)

Re: Reverse Engineering MacOS High Sierra Supplemental Update

#12

Post by emsy »

IncRnd wrote: post #4

if ((err = SSLHashSHA1.update(&hashCtx, &signedParams)) != 0) goto fail; goto fail; /* MISTAKE! THIS LINE SHOULD NOT BE HERE */ if ((err = SSLHashSHA1.final(&hashCtx, &hashOut)) != 0) goto fail; History repeats itself in ever stranger ways. While the effects are different, the original mistake can be quite similar.

You don't even need static analysis (which they should use too) but just a strict coding style check that enforces curly braces. Should've happened after the first incident!

Re: Reverse Engineering MacOS High Sierra Supplemental Update

#13

Post by bsaul »

Seems like Unit testing really is NOT popular at all at Apple. I mean, they release a framework for storage, and they don’t even unit test security related functions ?

Tdd introduced the wrong idea that unit tests should be all or nothing. I think it’s not. I unit test only the most critical parts of my programs (and only if there are), and i see value in it.

Re: Reverse Engineering MacOS High Sierra Supplemental Update

#15

Post by colinzoy »

1. to me the most important question is: why on earth can the disk password be retrieved in clear text in the first place ?!

2. also: the buggy version will be able to show disk passwords forever, until the encryption scheme is changed. macOS native encryption is useless until then (but given 1., it might already have been for some time).

Re: Reverse Engineering MacOS High Sierra Supplemental Update

#16

Post by danra »

It's pretty insane this sort of bug either passed or didn't even go through code review and testing at Apple, a company which has approximately infinite resources and whose marketing pitch is making high-quality products.

I loved Apple products and still do, but as both a user of and a developer for their systems, I feel the quality has been steadily going downhill the last few years.

Re: Reverse Engineering MacOS High Sierra Supplemental Update

#17

Post by nicolasd »

bsaul wrote: post #13

Seems like Unit testing really is NOT popular at all at Apple. I mean, they release a framework for storage, and they don’t even unit test security related functions ? Tdd introduced the wrong idea that unit tests should be all or nothing. I think it’s not. I unit test only the most critical parts of my programs (and only if there are), and i see value in it.

Serious question: How would you suggest to write a test that prevents this? You wouldn't make an assertion on clearTextPassword !== passwordHint. While developing I would think "Who will ever do that? That would be insane".

But yes, even if there is no test, this should have been caught in code review or latest when testing the OS.

Re: Reverse Engineering MacOS High Sierra Supplemental Update

#18

Post by a_t48 »

colinzoy wrote: post #15

1. to me the most important question is: why on earth can the disk password be retrieved in clear text in the first place ?! 2. also: the buggy version will be able to show disk passwords forever, until the encryption scheme is changed. macOS native encryption is useless until then (but given 1., it might already have been for some time).

(1) is the bug - you aren't supposed to be able to. the password was copied into a cleartext hint field.

(2) you should be fine if you reset your password and hint.

Re: Reverse Engineering MacOS High Sierra Supplemental Update

#19

Post by tempodox »

emsy wrote: post #12
IncRnd wrote: post #4

if ((err = SSLHashSHA1.update(&hashCtx, &signedParams)) != 0) goto fail; goto fail; /* MISTAKE! THIS LINE SHOULD NOT BE HERE */ if ((err = SSLHashSHA1.final(&hashCtx, &hashOut)) != 0) goto fail; History repeats itself in ever stranger ways. While the effects are different, the original mistake can be quite similar.

You don't even need static analysis (which they should use too) but just a strict coding style check that enforces curly braces. Should've happened after the first incident!

I find it jaw-dropping how fashionable cowboy coding still is, especially in C family languages.
Post reply on HN