Through one of my colleagues I learned yesterday about the infamous iOS SSL bug that resulted in iOS devices accepting any certificates whether they were correct or incorrect. This could allow an attacker or man in the middle to eavesdrop or intercept traffic theoretically secured through, for example, https.

I am not going to write today about what the bug does and the potential impact. I will, however, highlight the fact that this bug seems to have gone unnoticed since iOS 6 in 2012, which is quite scary. If you are interested in the bug and more details on the media burst it has created, you can read about it here, here or here.

I want to focus specifically on what the bug was. This bug that has terrifying consequences is just a basic and simple human mistake, probably originated at a classic copy-paste of code. Let’s look at the code, which I found here and is, in turn, from the Apple’s published open source code (which, obviously, is already fixed).

static OSStatus
SSLVerifySignedServerKeyExchange(SSLContext *ctx, bool isRsa, SSLBuffer signedParams,uint8_t *signature, UInt16 signatureLen)
{
    OSStatus        err;
    ...

    if ((err = SSLHashSHA1.update(&hashCtx, &serverRandom)) != 0)
        goto fail;

    if ((err = SSLHashSHA1.update(&hashCtx, &signedParams)) != 0)
        goto fail;
        goto fail;

    if ((err = SSLHashSHA1.final(&hashCtx, &hashOut)) != 0)
        goto fail;
    ...

fail:
    SSLFreeBuffer(&signedHashes);
    SSLFreeBuffer(&hashCtx);
    return err;
}

Essentially, after the second check, the line of code that goes to fail is repeated. Therefore, the third and last check for the hash is never executed and this subroutine always goes to fail. As you can see, this is not the fanciest bug ever. On the contrary, it is most likely a classic copy-paste human error.

I have always been very particular when coding in C and Java and I ALWAYS use {} for condition statements. So, although this

if ((err = SSLHashSHA1.update(&hashCtx, &signedParams)) != 0)
    goto fail;

is exactly the same as this

if ((err = SSLHashSHA1.update(&hashCtx, &signedParams)) != 0){
    goto fail;
}

using the latter would have avoided this bug. So, long story short, the conclusion is: always use {}. And a related corollary, vi and vim for Unix are cool and you look very cool and smart using them, but always use an IDE when coding. They are very smart and helpful. An IDE would have highlighted in bright annoying yellow the third hash check and labeled it as “dead code“. If you are a Unix user, I recommend Eclipse.

Advertisements