Android Bug Superior to Master Key

http://www.saurik.com/id/18

114 21 13 years ago

kevingadd• 13 years ago

It's interesting to see that the old reader class had a method 'readShortLE' that read an unsigned LE short and returned it as a signed integer; the 'readShort' being called in the new buggy code reads a signed short and returns it as a signed integer. Unfortunate misuse of naming (in the first case, naturally). I certainly would not have guessed the difference were I just reading the code.

It's unfortunate that this breakage all started because the original (correct, it seems?) code was too slow due to method call overhead. If people hadn't felt the need to optimize, this bug probably wouldn't be there - but I guess the Dalvik VM can't inline in that scenario, or something.

saurik3• 13 years ago

The code they have right now is slower with respect to method calls than all previous implementations. The C code worked. The low-memory Java that replaced it worked. The Java version that used less method calls worked. It was the person who then came in later whose job was not optimization that broke the reader while trying to fix a correctness bug.

One argument is that the code was now sufficiently complex due to the optimization that it was bound to confuse him. Another is that the other bug (the CRC one) was caused during the optimization sweeps that could have been handled elsewhere. (So your point to me largely stands, but I wanted to correct the timeline. I agree that spending time fixing inlining to help the entire codebase would have stopped the timeline of events that led to here, possibly not just on accident.)

MrKurtz• 13 years ago

The folks at Android Police posted about it weeks ago: http://www.androidpolice.com/2013/07/11/second-all-access-ap...

It also was already patched. Good write up though.

britta• 13 years ago

saurik explains in his introduction why his article is different (and potentially worth reading even if you've read other articles):

Later articles were written about this bug, each examining the same exploit technique, coming to the conclusion that this vulnerability offered less abilities than the Master Key exploit, due to very narrow and unlikely requirements that the original signed application package must satisfy.
In this article, I document different exploit techniques for this bug that do not have these limits; in fact, the second exploit described here provides much more capability than the Master Key exploit, allowing arbitrarily complex packages to take on the signatures from arbitrary signed packages. Finally, I look at the history of this bug in Android, showing when it was introduced.

saurik3• 13 years ago

Correct. In fact, in my section "Serious Limitations", I directly reference the article from Android Police and quote from its at-the-time understanding of the vulnerability.

agent1233• 13 years ago

A commit to a repository does not equal a patch to the half billion vulnerable devices in the field.

brokenparser2• 13 years ago

Yes, captain obvious? What do you suggest?

cremnob• 13 years ago

Open always wins

MichaelGG• 13 years ago

Perhaps interesting historical from Gosling on why Java has such limited primitives:

"almost no C developers actually understand what goes on with unsigned, what unsigned arithmetic is. Things like that made C complex. The language part of Java is, I think, pretty simple"

I do agree with the sentiment. All that type coercion in C is insane. But removing unsigned doesn't seem like the fix. Perhaps if languages stopped doing automatic conversions that'd solve the problem.

ajross• 13 years ago

I think the real problem with the logic is that it equates "reasoning about unsigned logic is hard" with "unsigned logic isn't required". Sadly it is. There are solutions that can only be expressed cleanly in unsigned logic, and others that can only be done signed.

Now, clearly there are more problems out there that need a sign bit and comparison operators that honor it than there are problems that don't. So Java made the "right" choice about which one to eject.

Still, hard code remains hard code, and there is no free lunch. Hard code is going to have bugs, and designing your language to make hard code harder (in the interest of making easy code easy) is probably a bad tradeoff in hindsight.

chacham152• 13 years ago

"almost no C developers actually understand what goes on with unsigned, what unsigned arithmetic is.

This seems like a large exaggeration unless I am one of such people. Correct me if I am wrong, but the arithmetic is simple (i.e. simple binary addition). What tricks people is when they start to mix unsigned and signed arithmetic. Even then, its only that they dont know which one is getting casted to the other (or not knowing the consequence of such a cast; e.g. how right shift is different with signed and unsigned numbers). That is a very simple thing that takes all of a google rtt to figure out.

mpyne• 13 years ago

A growing concern IMO is that there are many situations in C where you can hit undefined behavior with signed arithmetic, where the behavior would have been perfectly defined (if undesirable) with unsigned logic.

barrkel2• 13 years ago

Far too many C programmers think that unsigned ints are an appropriate type to use simply because the quantity in question can never be negative, e.g. the length of a list or the size of a memory allocation. What they fail to appreciate is that almost inevitably such values will get used in subtraction operations down the road, and then your chances of having a corner-case bug grow exponentially.

I doubt more than 1% of non-security-hardened extant C programs are completely safe when any unsigned quantity in the program may acquire a value with the high bit set.

jfb• 13 years ago

Speaking as someone with no Android or security background, this was an exceedingly well-written article. Big thumbs-up to saurik.

Scaevolus• 13 years ago

By comparison, Windows 8 app packages (.appx) signatures involve signing hashes of the entire zip file minus the signature file itself. http://blogs.msdn.com/b/windowsappdev/archive/2012/12/04/des...

If anyone knows how iOS .ipa packages are signed, I'd love to hear it.

saurik3• 13 years ago

Interestingly, they are doing both; this is similar to how SignApk (which is used to sign Android update.zip files) works. Microsoft's first layer of "sign all of the files" looks very similar to jarsign: both embed files inside of the zip file that include the signature, and both have a signature that signs a manifest that in turn is a list of hashes.

The only differences at that level of the signature process (before the whole file signature) are that 1) jarsign has one further indirection (whose purpose is to allow some metadata to be hashed, as well as to support multiple people signing different parts of the file) and 2) the Microsoft manifest signs blocks of files, not entire files (for efficiency).

That article does not describe the second layer of signature verification (the one that signs the whole file) but does mention it a few times. FWIW, as I bring up in an aside of my article, Android's update.zip files made a mistake in how they attached the signature to the end of the zip file, and ended up with a signing bug back in 2008 ;P.

Android Bug Superior to Master Key

Related Stories