
Reverse Engineering MacOS High Sierra Supplemental Update - nkkollaw
https://cocoaengineering.com/2017/10/08/reverse-engineering-macos-high-sierra-supplemental-update/
======
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.

~~~
veidr
I think you might be looking at the past through rose-colored glasses. Apple's
_always_ had horrible show-stopping bugs, in every single OS X / macOS
release[1], from when simple operations like updating iTunes or enabling the
OS's Guest Mode might delete your home directories, to this latest failure.

I suspect one reason is that they like one-person teams so much. I like one-
person projects too! But there is always pain when you have to hand such a
project to a new person, because the old one quit, or died, or whatever.

Overzealous secrecy even when not warranted for any actual business-relevant
reason also probably inhibits software quality. As does change for change's
sake.

[1] EDIT: except _maybe_ Mac OS X 10.6? Does anybody remember any critical
bugs in that one? I think that might be the unicorn.

~~~
rusk
_except maybe Mac OS X 10.6_

Snow Leopard. What a wonderful high-point in Desktop OSes that was. Seriously
considering rolling back one or two of my old computers, even just to have a
living breathing reference-point.

~~~
mitchty
It too had issues, this is the rose colored windows 7/xp/2000 glasses release
for mac fans.

~~~
rusk
All software has issues. Snow Leopard, and Mountain Lion were high points
absolutely.

------
blinkingled
> Apparently, Disk Utility and command line diskutil use different code paths.
> StorageKit does not appear as a direct dependency of diskutil. [snip] This
> duplication in what’s more or less the same functionality, while sometimes
> justified, certainly increases the opportunity for bugs.

To me this is the more problematic part - good design would use same code
paths as much as possible for the GUI app and the command line one - the UI
code in this case will differ but there should really be no need for diskutil
and Disk Utility to use duplicate code for storage functions.

~~~
rtpg
Yeah it seems bonkers to me that an OS would ship with two sets of disk
management libraries. At worst, at least try to make one depend on the other.

~~~
pducks32
I can’t believe I’m actually arguing this but devils advocate I guess: in this
instance if they both used the same library then the command line would also
have this problem. In this case at least there was a work around in that you
could use the cli.

------
dep_b
I don't like the old style NSDictionary way of packing information that so
many API's use. First of all they're lacking an enforcement for the type of
the value they're expecting (sometimes leading to crashes if you get them
wrong) and the discoverability of possibilities is less easy than with a
struct or object.

Still, passing in password twice would still be a possible mistake. The only
thing that would help in my opinion would be types that can be applied to
primitives, a bit like F# does. So you can declare certain Doubles to be
Miles, Kilometers, Meters and so on. Or a certain type of string can be of
type DatabaseID or a Password.

~~~
pducks32
Yea I was thinking about this too! We can talk all day about static typing vs
productivity but if they had a basic string type called PasswordHint and
Password then maybe (maybe!) this wouldn’t happen.

~~~
UnoriginalGuy
I'd leave passwordHint as a string; but password... That might be an
interesting type. Particularly if it zeroed out memory as a freebie in the
destructor. Actually that was my take-away from this article, the "maybe in MY
projects I should wrap passwords in a special type to stop me typo-ing the raw
password to output."

------
danielsamuels
> did the QA engineer test setting a password _and_ a password hint?, easily
> forgettable on a tight deadline

Or they used 'asd' as both the password and the password hint and therefore it
looked ok.

~~~
sschueller
What kind of amateur uses the same entry for two different fields? In the
early days of coding your entries going into the wrong place is something that
will happen to you.

~~~
justinjlynn
what kind of amateur does manual testing of basic functionality?

------
tempodox
Installing a new Mac OS on a machine you care for has become an absolute no-go
area. I wait for at least half a year and then only install in a place that I
can trash in case I still see problems.

~~~
rjzzleep
I'm not sure why you say "has become".

In the circles I've been it has been common practice to wait for the .2(but
minimum .1) release before it goes onto real work machines. The early Mac OS X
releases were even worse.

Even if for some reason the OS itself was fine by itself, they almost always
broke compat with a bunch of 3rd party applications.

EDIT: typo

~~~
Doctor_Fegg
Exactly. "Never buy the first version of any new Apple product", whether
hardware or OS, has been well-known for at least 10 years:
[https://www.engadget.com/2006/06/03/why-first-generation-
app...](https://www.engadget.com/2006/06/03/why-first-generation-apple-
products-suck/)

(That said, I've found my first-gen Apple Watch to be a great product. First-
gen iPad is a paperweight though...)

~~~
madeofpalk
Never buy the first version of _any_ new product

~~~
jackhack
Mostly, that's true. Or at least understand you're on the edge. Cutting or
bleeding, it's hard to say ahead of time but usually pretty obvious after a
week of real-world use.

I respect Apple, they take chances. Sometimes it pays off. Off the top of my
head:

Products where 1st generation was amazing: Apple ][

    
    
      iPod
    
      Lisa
    
      iPhone
    
    

Products where 1st gen was almost unusable, but v2+ was amazing:

    
    
      Macintosh (Amazing, but frequent disk swapping was an issue.)
    
      Messagepad (first gen soured the press and the public due to connectivity issues. Modem vs cellular.)
    
    

DOA: Apple 3 (anyone else remember the service bulletin that asked users to
slam it on the desk to reseat the chips?)

    
    
      Pippin

~~~
TheAceOfHearts
Original iPhone didn't have 3G, which many definitely considered to be a huge
flaw. It also didn't have native applications.

~~~
r00fus
It's not a huge flaw, as the mobile 3G (esp. AT&T's) networks weren't that
good in 2007.

The fact that you could get (edge speed) unlimited internet for $20/iPhone was
pretty amazing. Same for Palm/Verizon was $45 and more for 3G.

Native apps showed up in jailbreaks which I installed literally months after
the release. The app install process was unparalleled.

------
feelin_googley
"This research has shone a light on some of the ways in which security patch
support provided by Apple for 'firmware' is quite different than the security
patch support they provide for 'software'."

"You can be software 'secure' but firmware vulnerable."

"The release QA on the FirmwareUpdate bundles is concerning."

[https://duo.com/assets/ebooks/Duo-Labs-The-Apple-of-Your-
EFI...](https://duo.com/assets/ebooks/Duo-Labs-The-Apple-of-Your-EFI.pdf)

"The advent of UEFI brought with it a far more 'modern' pre-boot environment
and 'finally' put an end to the many years of legacy workarounds that had to
be applied to the aging IBM BIOS 'standard', providing a common, uniform and
higher-level platform to 'innovate' on."

" _However_ , that uniformity and accessibility also _opened the door_ to far
more generic and useful pre-boot environment attack opportunities."

------
vbezhenar
Why they are duplicating StorageKit code? If diskutil was using StorageKit,
it's more likely that someone would find this bug (because apparently everyone
in Apple uses diskutil instead of GUI).

------
mark_l_watson
Sorry if this is off topic, but I ran into another serious bug in High Sierra
that I think is a result of their de-duplicating of files feature: I had about
30 GB of family videos and pictures on OneDrive and wanted a second copy on
Dropbox. As I copied them I noticed that the disk space did not decrease, a
good thing to have file de-duplication. All was well for a long while until I
decided to have OneDrive and DropBox not keep my family videos and pictures on
my laptop.

After DropBox and OneDrive deleted these files, I never got the 30 GB of free
space back.

Apple should provide an option in the Disk Utility to check for files that
don’t have references and free the space.

------
intellix
the good thing is, you know that when they break something, it'll probably
stay broken for a couple more releases and a few years. Like how my mouse Y
axis is reversed when I restart... for about 2 years now

~~~
andai
I had a mouse that did just that. Nothing I tried could fix it.

I gave up and took the batteries out, and forgot about it for a few weeks.

When I tried it again it worked, and works fine ever since.

------
rconti
The Supplemental Update took _ages_ to install on my SSD-equipped 5k iMac
(it's only about a year old). In fact, I'm quite sure it took longer to
install than High Sierra itself, which is quite an achievement because the
High Sierra install involved upgrading my filesystem to APFS!

Maybe I just found myself more annoyed than usual because I clicked "try later
tonight" for the supplemental update prompt when using the computer the day
before, and then when I tried to use the computer in the morning, it went
about its installing business which featured 2 backwards-moving progress bars
and took at least 20 minutes, while I sat around twiddling my thumbs waiting
to use my computer for what should have been a simple task.

I'm not sure if this is normal behavior when you postpone an update and then
wake from sleep later, but I've never seen what effectively turned into an un-
prompted forced install before.

------
devdoomari
does this mean that all passwords for osx-encrypted-drives have been
'recoverable'? e.g. if I created an encrypted drive using El Capitan, someone
else can crack my drive's password without even cracking a password-hash?

Or is there a bug also in high-sierra's 'create-encrypted-disk' functionality?
(but not in lower-versions)

~~~
futurix
Only in the passwords created using GUI version of the new Disk Utility
(command line is safe).

~~~
devdoomari
thank you! thought I was screwed...

------
abalone
So the lesson here is, it’s a good idea to try to have your GUI and command
line use the same code paths. Looks like they created a separate API in
StorageKit just for Disk Utility and got sloppy with unit test.

------
SkyMarshal
Off topic but related, anyone know of any active OS X hacking/tweaking forums
or communities? I have an I/O issue with a peripheral I'm trying to isolate,
could use some expert help.

~~~
sethhochberg
www.tonymacx86.com is where a lot of the hackintosh community gathers. Plenty
of folks in the forums there who understand the kernel / IO / driver stack.

------
jagger27
What's interesting to me is that the version of Disk Utility in the High
Sierra installer doesn't have this issue.

------
ForFreedom
So, is it safe to install High Seirra since it was a low sierra couple of days
ago?

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

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

~~~
eru
Yes, with 'example based testing' this is hard to come up with. With property
based testing, it's not so hard to test this kind of UI things:

You test a UI by basically throwing sequences of interactions at it. Some of
the properties you'd want to assert:

* Given two interaction sequences that only differ in what they do to the password hint field in the UI, the result should only differ differ in the returned password hint. (Or alternatively neither should finish the UI dialogue.)

* As a dual: two interaction sequences that share the same interaction with the password hint field should yield the same password hint field. (In this case it is acceptable for them to differ in whether they actually finish the dialogue.)

Those two are fairly generic, so you can imagine setting that up as a general
framework for all your data input fields in your UIs. It should work backwards
as well, eg to assert that eg the UI should look the same no matter what
password (/ password hash) is stored, to make sure you are not leaking any
information.

See eg [https://fsharpforfunandprofit.com/posts/property-based-
testi...](https://fsharpforfunandprofit.com/posts/property-based-testing/) for
more background, and [http://hypothesis.works/articles/incremental-property-
based-...](http://hypothesis.works/articles/incremental-property-based-
testing/) for a real world example.

As for 'should have been caught in code review': yes, but humans are fallible
and they should get all the help we can give them. To see for yourself, have a
look at the example (a simple runlength encoding) at the top of
[http://hypothesis.readthedocs.io/en/latest/quickstart.html](http://hypothesis.readthedocs.io/en/latest/quickstart.html)
and see whether you can spot the obvious error just by reviewing the code.

~~~
nicolasd
Thanks for the detailed answer and coming up with a property-based test that
is not a "1+1=2" example :)

~~~
eru
I think I might have stumbled upon a new category of common properties here.
(At least new to me.) Basically, test that some subset of your output
variables is a function of a specific subset of your input variables. And
conversely, test that some subset of your output variables is independent
(under some conditions) of some subset of your input variables.

My go-to example for property based testing isn't addition, but idempotence.
Idempotence is a concept well known from REST apis to the general programming
public, and also in sorting or clean up data. And it's easy to state in code,
eg: sorted(sorted(x)) == sorted(x).

------
IncRnd

      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.

~~~
emsy
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!

~~~
eru
A coding style check _is_ static analysis.

(But that's beating a straw man, of course: with that expanded definition your
comment just because "you don't even need anything more than the most
primitive forms of static analysis".)

~~~
emsy
I think the point I was trying to make was: If they don't even check for
rudimentary things like coding style, can we expect them to check for more
sophisticated sources of issues (e.g. memory corruption).

~~~
eru
Amen.

------
sparkpeasy
Scary to think that the reveal of all passwords on MacOS shoulders on one
person's accidental copy + paste of one line.

~~~
saagarjha
It’s not a reveal of _all_ the passwords, certainly. It’s only disclosing the
disk’s password, and only in the situation provided.

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

~~~
chaosite
If I understand the blog post correctly, the bug was in the software that
creates (or maybe mounts? Not too clear about that) the encrypted volume. It
needs to have the clear-text password in order to function.

Then it proceeded to set the clear-text password as a password hint due to the
bug.

------
atas
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 spotting this and other things. But aren't unit
tests a duplication of your code, already?

UPDATE:

Maybe I should have stated my last question differently. I meant that in the
context of checking that the data is correct, it would be the same as writing
the duplicate code from scratch.

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

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

