Reverse Engineering macOS High Sierra Supplemental Update

Reported by Matheus Mariano, a Brazilian software developer, a programming error was discovered in Apple’s most recent operating system, High Sierra, that exposed passwords of encrypted volumes as password hints. A serious bug that quickly made the headlines in technology websites everywhere.

disk-utility-password-prompt-800x367

The dreaded password hint bug: Here, “dontdisplaythis” is the actual password.

 

Apple was prompt to provide macOS High Sierra Supplemental Update to customers via the App Store, and ensured that every distribution of High Sierra in their servers included this update.

I decided to apply a binary diffing technique to the update to learn more about the root cause of this bug and hypothesize about how the defect could have been prevented.

Inspecting the 51MB package, we can see that there are changes in the Disk Utility and Keychain Access apps, and also in related frameworks and command line tools:

Screen Shot 2017-10-08 at 11.53.25 AM

This post will focus only on the password hint bug, so our first step is to extract Applications/Utilities/Disk Utility.app/Contents/MacOS/Disk Utility  and to compare it with the same binary from a stock macOS 10.13 High Sierra. For this, I have written an Emacs extension that launches IDA whenever I load a Mach-O file in a buffer, generates a SQL database with information about the decompiled functions, loads the patched binary, and finally outputs a diff generated by Diaphora. This technique is useful for deconstructing binaries that have been updated by a minor patch release because there are usually just a few changes and common heuristics work well.

The diff between both versions of the Disk Utility binary revealed no differences in the decompilation:

VirtualBox_Windows 10_08_10_2017_13_20_16

That usually means that the only substantial changes reside in one of the linked frameworks. The most interesting one for this investigation is StorageKit, a private Apple framework that exposes APFS functionality to Disk Utility. It has two parts: a client library and a daemon, storagekitd. The client connects to the daemon using an Apple standard XPC mechanism. The daemon executes the operations (represented as subclasses of NSOperation) that the client demands. Here’s an interesting usage of StorageKit inside Disk Utility:

VirtualBox_Windows 10_08_10_2017_13_42_24

Reference to a StorageKit structure from controller code in Disk Utility.

This is part of the code that runs when you add a new APFS volume from the Disk Utility interface (concretely, the controller responsible for managing the new volume sheet).

Diffing StorageKit provided much more interesting results:

VirtualBox_Windows 10_08_10_2017_14_00_13

​​​[SKHelperClient addChildVolumeToAPFSContainer:name:caseSensitive:minSize:maxSize:password:passwordHint:progressBlock:completionBlock:] was one of the functions modified by the supplemental update. Inspecting the differences in decompilation revealed the actual bug:

VirtualBox_Windows 10_08_10_2017_14_15_16

In the picture above, the old, vulnerable, StorageKit is diff’d against the updated one. Removed lines removed are depicted in red, added lines in green, and changes in yellow. The above function basically creates an instance of NSMutableDictionary (Cocoa’s representation of a hash table) and fills it with information about the volume. This dictionary is passed to addChildVolumeToAPFSContainer:optionsDictionary:handlingProgressForOperationUUID:completionBlock: as the optionsDictionary argument.

The most interesting keys in the dictionary are kSKAPFSDiskPasswordOption and kSKAPFSDiskPasswordHintOption, which are responsible for storing the password and the password hint, respectively. The bug is that the same variable, which contains the password, (represented in the decompilation as the same virtual register, v50) was used as value for both keys in the dictionary, meaning that the clear password was incorrectly sent as a password hint via XPC. In reconstructed Objective-C code, the bug would be something like this:

NSMutableDictionary *optionsDictionary = [NSMutableDictionary alloc] init];
[...]
optionsDictionary[kSKAPFSDiskPasswordOption] = password;

optionsDictionary[kSKAPFSDiskPasswordHintOption] = password;

Here’s the corrected function from the supplemental update:

Updated

Note that the correct variables for the password and the password hint are set.

This is an example of a common category of bugs where code with a common structure is copied and pasted but the developer forgets to make every required modification and consequently there’s a fatal change in behavior. If you are curious, this blog post shows you more examples of “Last Line Effect” bugs in open source software.

It’s important to emphasize that, although this particular dictionary is not stored anywhere (it’s simply used to pack the information that is sent to storagekitd), the fact that the password was sent incorrectly as password hint meant that storagekitd trusted its client and stored it as clear text, thinking it was a password hint.

Why did the bug not reproduce when using the command line?

This is a common question. Apparently, Disk Utility and command line diskutil use different code paths.  StorageKit does not appear as a direct dependency of diskutil, or in the transitive closure of its dependencies. Here’s otool -L output:

/usr/lib/libcsfde.dylib (compatibility version 1.0.0, current version 1.0.0)/usr/lib/libcsfde.dylib (compatibility version 1.0.0, current version 1.0.0) /usr/lib/libCoreStorage.dylib (compatibility version 1.0.0, current version 1.0.0) /System/Library/Frameworks/Foundation.framework/Versions/C/Foundation (compatibility version 300.0.0, current version 1443.14.0) /System/Library/Frameworks/IOKit.framework/Versions/A/IOKit (compatibility version 1.0.0, current version 275.0.0) /System/Library/PrivateFrameworks/DiskManagement.framework/Versions/A/DiskManagement (compatibility version 1.0.0, current version 1.0.0) /System/Library/Frameworks/DiscRecording.framework/Versions/A/DiscRecording (compatibility version 1.0.0, current version 1.0.0) /usr/lib/libncurses.5.4.dylib (compatibility version 5.4.0, current version 5.4.0) /System/Library/Frameworks/DiskArbitration.framework/Versions/A/DiskArbitration (compatibility version 1.0.0, current version 1.0.0) /usr/lib/libicucore.A.dylib (compatibility version 1.0.0, current version 59.1.0) /usr/lib/libobjc.A.dylib (compatibility version 1.0.0, current version 228.0.0) /usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1252.0.0) /System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation (compatibility version 150.0.0, current version 1443.13.0)

This duplication in what’s more or less the same functionality, while sometimes justified, certainly increases the opportunity for bugs.

How could this have been prevented?

There’s two engineering practices that help with bugs like this (but do not eradicate them completely):

Unit testing

Unit testing is the practice of creating software tests that exercise a single unit in a computer program, where “unit” is typically a class or module. Effective unit testing requires sensing outputs reliably and asserting that they are expected, so side effects from functions complicate unit testing a bit. In this particular bug, the side effect is the communication with the XPC service, so separating the logic that creates the dictionary from the part that communicates with the service would help. When a software design is not easily testable, companies rely excessively on manual testing, which is not a very effective way of testing, given the high number of combinations that is typical in modern software (did the QA engineer test setting a password *and* a password hint?, easily forgettable on a tight deadline).

Code review

Code review is the practice of reviewing code before or after it lands the main development branch in a software project. Code reviews should always be small, so that the reviewer’s attention is focused and can suggest better improvements and even spot bugs like this. A “last line” bug can easily be ignored if it’s part of a huge code review.

Conclusion

An unfortunate bug in macOS High Sierra stained a bit its generally well-received debut, and from this root-case analysis we can learn what happened exactly and how good software development practices (including testable design and strict code reviews) can help reduce the chance that this kind of problems happen again in the future.

 

Advertisements
This entry was posted in Reverse Engineering. Bookmark the permalink.

16 Responses to Reverse Engineering macOS High Sierra Supplemental Update

  1. Pingback: A Look Inside That macOS High Sierra Supplemental Update – 512 Pixels

  2. orionblastar says:

    Is this bug shared with the Darwin, Xnu, and BSD Unix codebases?

    Did Apple goof and it got into shared code?

    Like

  3. Pingback: Reverse Engineering macOS High Sierra Supplemental Update | ExtendTree

  4. Pingback: Reverse Engineering macOS High Sierra Supplemental Update | A1A

  5. Kevin H says:

    Very informative and in-depth look! Thank you!

    Like

  6. Pingback: Michael Tsai - Blog - Encrypted APFS Volume’s Password Exposed as Hint

  7. Pingback: Reverse Engineering macOS High Sierra Supplemental Update - ZRaven Consulting

  8. myklwelch says:

    Why are they storing the password at all? Isn’t that the real bug?

    Like

    • mmuncy1989 says:

      You have to store the password at some point. You have to keep it in memory when the user could still be typing. You cannot hash it until fully formed. The bug is a simple misuse of one variable. In my experience you could have 10 developers review and still risk missing it.

      Like

    • I have the same question – why did OSX keep a record of the plaintext password at all? It feels like there’s a big issue hiding behind that question.

      Like

  9. Pingback: macOS High Sierraで設定したパスワードがパスワードのヒントに平文のまま表示されてしまう不具合の原因をリバースエンジニアリングにより解説した記事が話題。 | AAPL Ch.

  10. Pingback: 【知识】10月10日 – 每日安全知识热点-安全路透社

  11. Pingback: 【知识】10月10日 – 每日安全知识热点 – 安百科技

  12. Pingback: OTR Links 10/12/2017 – doug — off the record

  13. Pingback: 【技术分享】对macOS High Sierra补充更新的逆向分析 – 安百科技

Leave a Reply to Nathan Arthur Cancel reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s