Bad Java decompilation means erroneous statement in research paper

Back in July 9, 2012, Martin Georgiev et al. published a paper entitled “The Most Dangerous Code in the World: Validating SSL Certificates in Non-Browser Software” (download) that points out broken SSL certificates validation in various applications and third-party libraries.

One of the case studied is the Chase mobile banking app for Android. It turns out the version studied by the authors was 2.5 or earlier, released on April 23, 2012. (The APK can be found on Android-Drawer.) In paragraph 10.1, the authors wrote:

“Decompilation and analysis of this app’s code show that it overrides the default X509TrustManager. The replacement code simply returns without checking the server’s certificate. The code below is the result of reverse-engineering, thus variable names and other details may differ from the actual code.”

While the first statement is true (X509Certificate.checkServerTrusted() is overriden), the second is false. The claim was made because of improper decompilation to Java:

public final void checkServerTrusted(X509Certificate[]paramArrayOfX509Certificate,    String paramString)
{
  if ((paramArrayOfX509Certificate != null)
    && (paramArrayOfX509Certificate.length == 1))
    paramArrayOfX509Certificate[0].checkValidity();
  while (true)
  {
    return;  // makes checkServerTrusted unreachable
    this.a.checkServerTrusted(paramArrayOfX509Certificate, paramString);
  }
}

The decompiled code is incorrect: the “while(true) { return; …” is a misconstruct that lets the reverse engineer believe that this.a.checkServerTrusted() is never called.

Unfortunately, the authors relied in part on this faulty piece of Java code to claim that:

“Note the unreachable invocation of checkServerTrusted. We conjecture that this was a temporary plug during development that somehow found its way into the production version of the app.

Chase [is] completely insecure against a man-in-the-middle attack.”

Note that the conclusion that Chase is insecure to MiTM attacks is not disputed here. Martin Georgiev confirmed that the app was tested and was vulnerable to such attacks.

The authors may have run out of time and probably skipped the Dalvik bytecode verification step.

In fact, the routine in question is pretty simple, and JEB decompiles it to a clean:1

Now, it looks clear that the original checkServerTrusted() gets called, if and only if the certificates’ array is null or the array does not contain just one certificate.

Decompilation is not a guaranteed process, but one should use professional tools to minimize exposure to bugs. A manual check of the low-level bytecode or assembly is also a requirement before making claims that a particular code path is or is not executed.

Thanks to Juliano Rizzo for pointing out this potential issue.

Leave a Reply

Your email address will not be published. Required fields are marked *

*