{"id":300,"date":"2016-05-05T12:52:11","date_gmt":"2016-05-05T11:52:11","guid":{"rendered":"http:\/\/babbacom.com\/?p=300"},"modified":"2016-05-06T11:59:26","modified_gmt":"2016-05-06T10:59:26","slug":"security-bug-in-dropbox-sample-code","status":"publish","type":"post","link":"https:\/\/babbacom.com\/?p=300","title":{"rendered":"Security bug in Dropbox sample code"},"content":{"rendered":"

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.<\/p>\n

private void InitializeCertPinning() {\r\n\u00a0\u00a0 \u00a0ServicePointManager.ServerCertificateValidationCallback = (sender, certificate, chain, sslPolicyErrors) => {\r\n\u00a0\u00a0 \u00a0\u00a0\u00a0 \u00a0var root = chain.ChainElements[chain.ChainElements.Count - 1];\r\n\u00a0\u00a0 \u00a0\u00a0\u00a0 \u00a0var publicKey = root.Certificate.GetPublicKeyString();\r\n\r\n\u00a0\u00a0 \u00a0\u00a0\u00a0 \u00a0return DropboxCertHelper.IsKnownRootCertPublicKey(publicKey);\r\n\u00a0\u00a0 \u00a0};\r\n}<\/pre>\n
This is because it isn’t strictly necessary, and actually introduces a security bug.<\/p>\n
My son<\/a>, 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.<\/p>\n
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<\/p>\n
private void InitializeCertPinning() {\r\n\u00a0\u00a0 \u00a0ServicePointManager.ServerCertificateValidationCallback = (sender, certificate, chain, sslPolicyErrors) => {\r\n        if (sslPolicyErrors != 0) return false;\r\n\u00a0\u00a0 \u00a0\u00a0\u00a0 \u00a0var root = chain.ChainElements[chain.ChainElements.Count - 1];\r\n\u00a0\u00a0 \u00a0\u00a0\u00a0 \u00a0var publicKey = root.Certificate.GetPublicKeyString();\r\n\r\n\u00a0\u00a0 \u00a0\u00a0\u00a0 \u00a0return DropboxCertHelper.IsKnownRootCertPublicKey(publicKey);\r\n\u00a0\u00a0 \u00a0};\r\n}<\/pre>\n
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.<\/p>\n
It is a good idea to use the corrected code (and I’ll add it in before releasing the next version of Mobile DMX<\/a>), because it ensures that the Dropbox certificate has been issued by an authority that is known by Dropbox to issue certificates for their services.<\/p>\n
Addendum: Turns out this code doesn’t work on Xamarin Android anyway. It throws an exception because the chain doesn’t contain any elements.<\/p>\n","protected":false},"excerpt":{"rendered":"
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.<\/p>\n","protected":false},"author":2,"featured_media":0,"comment_status":"open","ping_status":"open","sticky":false,"template":"","format":"standard","meta":{"footnotes":""},"categories":[7],"tags":[42,43],"class_list":["post-300","post","type-post","status-publish","format-standard","hentry","category-programming","tag-dropbox","tag-security"],"_links":{"self":[{"href":"https:\/\/babbacom.com\/index.php?rest_route=\/wp\/v2\/posts\/300","targetHints":{"allow":["GET"]}}],"collection":[{"href":"https:\/\/babbacom.com\/index.php?rest_route=\/wp\/v2\/posts"}],"about":[{"href":"https:\/\/babbacom.com\/index.php?rest_route=\/wp\/v2\/types\/post"}],"author":[{"embeddable":true,"href":"https:\/\/babbacom.com\/index.php?rest_route=\/wp\/v2\/users\/2"}],"replies":[{"embeddable":true,"href":"https:\/\/babbacom.com\/index.php?rest_route=%2Fwp%2Fv2%2Fcomments&post=300"}],"version-history":[{"count":5,"href":"https:\/\/babbacom.com\/index.php?rest_route=\/wp\/v2\/posts\/300\/revisions"}],"predecessor-version":[{"id":305,"href":"https:\/\/babbacom.com\/index.php?rest_route=\/wp\/v2\/posts\/300\/revisions\/305"}],"wp:attachment":[{"href":"https:\/\/babbacom.com\/index.php?rest_route=%2Fwp%2Fv2%2Fmedia&parent=300"}],"wp:term":[{"taxonomy":"category","embeddable":true,"href":"https:\/\/babbacom.com\/index.php?rest_route=%2Fwp%2Fv2%2Fcategories&post=300"},{"taxonomy":"post_tag","embeddable":true,"href":"https:\/\/babbacom.com\/index.php?rest_route=%2Fwp%2Fv2%2Ftags&post=300"}],"curies":[{"name":"wp","href":"https:\/\/api.w.org\/{rel}","templated":true}]}}