Reinventing the Wheel is not Best Practice

Having already been disappointed in a mobile banking application in the past, I've decided to take a closer look at another one. I've started by looking at the shared preferences and other miscellaneous files for the application, see if it stores anything confidential, but that was not the case. The next step was to try and man-in-the-middle the application, however, all connections failed, even though I had a trusted CA installed. While certificate pinning is a best practice, I did not expect it to be implemented, based on previous experience. So far, so good.

The next step was to grab the APK off my device and disassemble it, mostly out of curiosity, to see how the certificate pinning is implemented, and also to check if it is hiding confidential credentials in a file under a path other than what I previously checked.

TLS without hostname verification

Looking at the networking part of the code, a specific class is responsible for making all the calls to the backend API of the bank. This code, unfortunately, has some interesting bits: (Note: the code is not copied from the disassembled source, instead recreated to demonstrate the original functionality.)

Before sending each request to the backend, the application first checks the connection. This check (done by verifyConnection) is very brief, it only connects to the host and retrieves its SSL certificate. This certificate is then validated with custom logic, and the result (whether it is deemed valid or not) is returned to sendRequest, which continues on with the normal procedure or stops with an error.

The problematic part here is that once the connection is deemed secure, the application will create a completely new connection, where the hostname verifier is set to a variable called DO_NOT_VERIFY, which is an instance of a custom HostnameVerifier implementation, which in turn accepts any name passed to it. It is important to note here, that TLS verfication is not turned off, only the name checking phase. The type of this vulnerability is classified as CWE-297.

As a result, it is possible to write a script for your MITM tool of choice, which ignores the first TLS connection (either based on the destination being the IP of the bank, or by inspecting the TLS ClientHello's SNI field) and then MITMs the second connection, since this is after verification, when the actual connection is happening. All you need to make sure of is to send a valid certificate, but it can be valid for any domain name.

Improper certificate validation

Digging deeper into the code, I've decided to take a look at their custom logic for certificate verification.

After getting the SSL certificate of the remote host, the certificate chain is built using the CertificateFactory.generateCertPath() API, and then the resulting CertPath object is converted to string, which looks something like this:

Since the hostname is something like bankname.ro, the searched string is CN=bankname.ro as a result. This is problematic, since you could get a certificate for bankname.ro.whatever.tk and it will still match. Note that some CAs would possibly flag this certificate request, but since we're not talking about a big US bank, it's unlikely that they will, and if they do, you can just try another one.

Encrypted secrets with key included

The application is using client certificates while communicating with the remote server. Unfortunately, the certificates are static files which are included for the production and development environments within the resources of the application. While the private keys of the certificates are encrypted, the decryption code is embedded for both in the logic of the application, in plain text for the development environment, and base64-encoded for the production one.

This is not particularly an issue, since knowing the private key of the client certificate won't make the connection any less secure, but it is still not a best practice, and an instance of CWE-259.

My first gut reaction was that this is a security theatre, however, I realized this would effectively prevent mass instances of MITM (such as by an enterprise firewall), since the firewall can establish a trusted connection with the device (given that the in-house CA is installed) but the bank servers would refuse the connection on their side, since it would be lacking the client certificate. While it's an effective way to defend against mass attacks, since the client certificates can be easily extracted, it does not prevent targeted attacks. Still, I applaud the bank's security team for coming up with this layer of protection.

Proposed fix

The fix for the first vulnerability could be to replace the DO_NOT_VERIFY (which is not a constant, but rather a variable pointing to an instance of a class implementing HostnameVerifier without performing any actual checks) with another implementation of HostnameVerifier which does at least make sure the hostname is part of the bank's actual domain names:

The first fix suggests that the use of the all-accepting custom hostname verifier was never meant to work on production, only on development environments. Whoops.

The second fix resolves my immediate concerns, however, it is still performing full-text search on the whole certificate chain information. Somewhat unlikely, however, it might still be possible to trick some CA into issuing a certificate, which has a description (or any field) containing a string like CN=bankname.ro,.

In summation, the bank definitely tried to make the application secure, it just missed a few logic errors. However, the communication could have been better, as to this day, I was not emailed by them at all, the whole process happened silently.