Tag: apple ssl coding conventions

double fail

Here is what is dubbed the ‘Apple Bug’:

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;
}

So what is this? If you ask me it looks like a combination of stupid coding conventions,  a merge artifact and not enough coverage in unit tests. 

Coding conventions

A lot of coding conventions will tell you that you should use braces around blocks even if a block is only a single line. Would it have helped? Perhaps. If it would result in code like this:

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

It looks funny but in this case the double fail would be harmless. And a good compiler would have warned about unreachable code.

Merge artifact

Could be. Could even be caused by changes in whitespace – who knows.

Test coverage

This is a conclusion that makes me worry. This kind of error should have surfaced in unit testing this function.