The #IAmRoot flaw
The recent "I Am Root" bug in MacOS (#1) brings to mind two of my favorite topics: functional results, and function naming. (Although this post is C-oriented, the concepts apply to any programming language.)
The bug:
An analysis from http://www.theregister.co.uk/2017/11/29/apple_macos_high_sierra_root_bug_patch
reads, in part:
1. Functional results
My guess is that the above is an example of the precise thing I frequently argue against:
having routines return 0 for success and 1 for failure (or vice versa) ... an extremely common technique, particularly on Unix/Linux.
Those values simply DO NOT HAVE such an implied meaning, and writing functions like that is asking for problems ... and sometimes you get what you ask for!
In this case, I suspect that some of the Apple code may have been:
if (od_verify_crypt_password (...the parameters)...) )
...foo...
...wherein the programmer (and reader) has no clue: does a "true" result (#2) mean it failed, or that it succeeded?
If, instead, od_verify_crypt_password been written to return a type of good_failed (#3) , then the programmer (and reader) would see:
if (od_verify_crypt_password (...the parameters)...) == FAILED)
...foo...
and would absolutely **KNOW** that 'foo' is executed when
the password verification fails, not when it succeeds.
Moral: if you don't know how a function returns a result, it's easy to call it incorrectly.
2. Function Naming
The name of the function odm_RecordVerifyPassword would appear to be a particularly poor one.
As a complete outsider, I might guess "odm" means something like "Online Dialog Manager", but that doesn't seem to quite fit here. Normally, I'd be hard pressed to come up with an acronym for "o", "d", "m" that is related to security checking, but some Google searching lead me to think that "od" might be "Open Directory" In that case, the prefix "od_" might be more appropriate.
The "odm_" prefix aside, and ignoring the ugly mixture of good naming style (using the underscore as a word separator) and the truly awful CamelCaseStyle (#4), let's look at the three main words in the function name: "Record", "Verify", and "Password".
English is an a terrifyingly complex language. As an example, the word "record", unfortunately, is both a noun and a verb. This means we have to ask what the words in the function name mean:
Is "record verify password" recording the result of previously attempting to verify a password (using "record" as a verb), or is it it a request to verify a record (data structure) of type "password"?
The reader can not know from just those three words ... and that means it's a bad name for the function.
Renaming od_verify_crypt_password would clarify its meaning, and would allow it to continue to be used as a simple 'boolean' without introducing the usual (section 1 above) confusion:
if (od_is_encrypted_password_correct (...))
...
In this case, "is..." makes it clear: you're asking a yes/no question. The "verify..." isn't clear, because one way of handling a failed verification is to abort or trap in some manner (and, absent an abort, the returns of 0 or 1 aren't inherently meaningful).
Moral: a good function name is critical for allowing the programmer, and reader, to understand what the function is intended to do.
In general, I highly recommend function names that are verb phrases ... that indicates that they *do* something! (E.g., "delete_file" instead of "unlink".)
Stan
sieler@allegro.com
---
#1. "Luckily", Apple's marketing policies accidentally protected me, since I have the latest Mac that isn't allowed to run HighSierra :)
#2. In C, any non-0 is considered "true" for an "if" statement. Although convenient, that design decision has lead to many programming errors (albeit not of the type discussed above). In a language like Pascal, where you *must* have something that evaluates to "true" or "false", similar errors are far less likely.
#3. In C:
typedef enum {FAILED, GOOD} good_failed;
In Pascal:
type good_failed (failed, good); {Isn't case insensitivity nice?!}
#4. CamelCase ... ifItWasSuchAGoodIdea, weWouldWriteThisWay. More on this in a future post.
The recent "I Am Root" bug in MacOS (#1) brings to mind two of my favorite topics: functional results, and function naming. (Although this post is C-oriented, the concepts apply to any programming language.)
The bug:
In some circumstances, perhaps a few less than many news report implied,
a non-privileged user could obtain root access on Macs running the
HighSierra version of macOS.
An analysis from http://www.theregister.co.uk/2017/11/29/apple_macos_high_sierra_root_bug_patch
reads, in part:
...
the security daemon opendirectoryd calls an internal function called odm_RecordVerifyPassword.
...
Seeing as that shadow hash lookup failed, opendirectoryd next tries to retrieve and check a crypt password for the account using od_verify_crypt_password. Weirdly, that function returns the value 0x1, signaling it was successful, and rather than bail out and deny access, the code falls through to function calls that upgrade the crypt password to a shadow hash and stores it for the account.
...
Reverse-engineered opendirectoryd code showing the heart of the bug, with od_verify_crypt_password returning 0x1 and screwing up the 0x0 check ... Source: Patrick Wardle, Mac security specialist and Synack chief researcher.
...
It appears that od_verify_crypt_password should fail (maybe it does and the check of the return code for 0x0 is just inverted?)
Or perhaps the call to odm_RecordVerifyPassword assumes can only be called in a validated/authenticated context?
My guess is that the above is an example of the precise thing I frequently argue against:
having routines return 0 for success and 1 for failure (or vice versa) ... an extremely common technique, particularly on Unix/Linux.
Those values simply DO NOT HAVE such an implied meaning, and writing functions like that is asking for problems ... and sometimes you get what you ask for!
In this case, I suspect that some of the Apple code may have been:
if (od_verify_crypt_password (...the parameters)...) )
...foo...
...wherein the programmer (and reader) has no clue: does a "true" result (#2) mean it failed, or that it succeeded?
If, instead, od_verify_crypt_password been written to return a type of good_failed (#3) , then the programmer (and reader) would see:
if (od_verify_crypt_password (...the parameters)...) == FAILED)
...foo...
and would absolutely **KNOW** that 'foo' is executed when
the password verification fails, not when it succeeds.
Moral: if you don't know how a function returns a result, it's easy to call it incorrectly.
2. Function Naming
The name of the function odm_RecordVerifyPassword would appear to be a particularly poor one.
As a complete outsider, I might guess "odm" means something like "Online Dialog Manager", but that doesn't seem to quite fit here. Normally, I'd be hard pressed to come up with an acronym for "o", "d", "m" that is related to security checking, but some Google searching lead me to think that "od" might be "Open Directory" In that case, the prefix "od_" might be more appropriate.
The "odm_" prefix aside, and ignoring the ugly mixture of good naming style (using the underscore as a word separator) and the truly awful CamelCaseStyle (#4), let's look at the three main words in the function name: "Record", "Verify", and "Password".
English is an a terrifyingly complex language. As an example, the word "record", unfortunately, is both a noun and a verb. This means we have to ask what the words in the function name mean:
Is "record verify password" recording the result of previously attempting to verify a password (using "record" as a verb), or is it it a request to verify a record (data structure) of type "password"?
The reader can not know from just those three words ... and that means it's a bad name for the function.
Renaming od_verify_crypt_password would clarify its meaning, and would allow it to continue to be used as a simple 'boolean' without introducing the usual (section 1 above) confusion:
if (od_is_encrypted_password_correct (...))
...
In this case, "is..." makes it clear: you're asking a yes/no question. The "verify..." isn't clear, because one way of handling a failed verification is to abort or trap in some manner (and, absent an abort, the returns of 0 or 1 aren't inherently meaningful).
Moral: a good function name is critical for allowing the programmer, and reader, to understand what the function is intended to do.
In general, I highly recommend function names that are verb phrases ... that indicates that they *do* something! (E.g., "delete_file" instead of "unlink".)
Stan
sieler@allegro.com
---
#1. "Luckily", Apple's marketing policies accidentally protected me, since I have the latest Mac that isn't allowed to run HighSierra :)
#2. In C, any non-0 is considered "true" for an "if" statement. Although convenient, that design decision has lead to many programming errors (albeit not of the type discussed above). In a language like Pascal, where you *must* have something that evaluates to "true" or "false", similar errors are far less likely.
#3. In C:
typedef enum {FAILED, GOOD} good_failed;
In Pascal:
type good_failed (failed, good); {Isn't case insensitivity nice?!}
#4. CamelCase ... ifItWasSuchAGoodIdea, weWouldWriteThisWay. More on this in a future post.
Comments
Post a Comment