Security bug in Dropbox sample code

If you have read the last entry on Dropbox login using the V2 API and compared it with the original sample code from Dropbox you may have noticed that I haven’t called the routine to initialise certificate pinning.

private void InitializeCertPinning() {
    ServicePointManager.ServerCertificateValidationCallback = (sender, certificate, chain, sslPolicyErrors) => {
        var root = chain.ChainElements[chain.ChainElements.Count - 1];
        var publicKey = root.Certificate.GetPublicKeyString();

        return DropboxCertHelper.IsKnownRootCertPublicKey(publicKey);
    };
}

This is because it isn’t strictly necessary, and actually introduces a security bug.

My son, a freelance internet security consultant, is staying with me at the moment, and spotted the bug when I asked him exactly what the code was doing.

If no callback is supplied a full range of checks is run on certificates, but if a callback is supplied it replaces the normal checks and it is necessary to check whether sslPolicyErrors is zero. The code should read

private void InitializeCertPinning() {
    ServicePointManager.ServerCertificateValidationCallback = (sender, certificate, chain, sslPolicyErrors) => {
        if (sslPolicyErrors != 0) return false;
        var root = chain.ChainElements[chain.ChainElements.Count - 1];
        var publicKey = root.Certificate.GetPublicKeyString();

        return DropboxCertHelper.IsKnownRootCertPublicKey(publicKey);
    };
}

He put together a man-in-the-middle attack, wrote it up and sent it to Dropbox, who quickly confirmed the error and gave him a nice bug bounty.

It is a good idea to use the corrected code (and I’ll add it in before releasing the next version of Mobile DMX), because it ensures that the Dropbox certificate has been issued by an authority that is known by Dropbox to issue certificates for their services.

Addendum: Turns out this code doesn’t work on Xamarin Android anyway. It throws an exception because the chain doesn’t contain any elements.