Exception-oriented Exploitation on iOS
Ian Beer (via John Gordon):
My guess is that the developer copy-pasted the code for the entire function then tried to add the extra level of indirection but forgot to change the third argument to the copyin call shown above. They built XNU and looked at the compiler error messages. XNU builds with clang, which gives you fancy error messages like this:
error: no member named 'recipes_size' in 'struct mach_voucher_extract_attr_recipe_args'; did you mean 'recipe_size'? if (copyin(args->recipes, (void *)krecipes, args->recipes_size)) { ^~~~~~~~~~~~ recipe_sizeClang assumes that the developer has made a typo and typed an extra ‘s’. Clang doesn’t realize that its suggestion is semantically totally wrong and will introduce a critical memory corruption issue. I think that the developer took clang’s suggestion, removed the ‘s’, rebuilt and the code compiled without errors.
[…]
Perhaps most importantly: I think this bug would have been caught in development if the code had any tests. As well as having a critical security bug the code just doesn’t work at all for a recipe with a size greater than 256. On MacOS such a test would immediately kernel panic. I find it consistently surprising that the coding standards for such critical codebases don’t enforce the development of even basic regression tests.
1 Comment RSS · Twitter
A well documented programming mistake and exploit.
Let me repeat what I commented on that blog:
One of my coding rules is to trace thru every code branch at least one, just to make sure that it works at all, even if I don't write persistent (unit) test code for it. If that had been done here, the bug would have been obvious as soon as the code segment for (sz >= MACH_VOUCHER_TRAP_STACK_LIMIT) would have been stepped through. Simple rule but very important and effective.
