Last active

Embed URL

HTTPS clone URL

SSH clone URL

You can clone with HTTPS or SSH.

Download Gist
View gist:9176925
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86
static OSStatus
SSLVerifySignedServerKeyExchange(SSLContext *ctx, bool isRsa, SSLBuffer signedParams,
uint8_t *signature, UInt16 signatureLen)
{
OSStatus err;
SSLBuffer hashOut, hashCtx, clientRandom, serverRandom;
uint8_t hashes[SSL_SHA1_DIGEST_LEN + SSL_MD5_DIGEST_LEN];
SSLBuffer signedHashes;
uint8_t *dataToSign;
size_t dataToSignLen;
signedHashes.data = 0;
hashCtx.data = 0;
clientRandom.data = ctx->clientRandom;
clientRandom.length = SSL_CLIENT_SRVR_RAND_SIZE;
serverRandom.data = ctx->serverRandom;
serverRandom.length = SSL_CLIENT_SRVR_RAND_SIZE;
if(isRsa) {
/* skip this if signing with DSA */
dataToSign = hashes;
dataToSignLen = SSL_SHA1_DIGEST_LEN + SSL_MD5_DIGEST_LEN;
hashOut.data = hashes;
hashOut.length = SSL_MD5_DIGEST_LEN;
-
- if ((err = ReadyHash(&SSLHashMD5, &hashCtx, ctx)) != 0)
+
+ if ((err = ReadyHash(&SSLHashMD5, &hashCtx)) != 0)
goto fail;
if ((err = SSLHashMD5.update(&hashCtx, &clientRandom)) != 0)
goto fail;
if ((err = SSLHashMD5.update(&hashCtx, &serverRandom)) != 0)
goto fail;
if ((err = SSLHashMD5.update(&hashCtx, &signedParams)) != 0)
goto fail;
if ((err = SSLHashMD5.final(&hashCtx, &hashOut)) != 0)
goto fail;
}
else {
/* DSA, ECDSA - just use the SHA1 hash */
dataToSign = &hashes[SSL_MD5_DIGEST_LEN];
dataToSignLen = SSL_SHA1_DIGEST_LEN;
}
hashOut.data = hashes + SSL_MD5_DIGEST_LEN;
hashOut.length = SSL_SHA1_DIGEST_LEN;
- if ((err = SSLFreeBuffer(&hashCtx, ctx)) != 0)
+ if ((err = SSLFreeBuffer(&hashCtx)) != 0)
goto fail;
- if ((err = ReadyHash(&SSLHashSHA1, &hashCtx, ctx)) != 0)
+ if ((err = ReadyHash(&SSLHashSHA1, &hashCtx)) != 0)
goto fail;
if ((err = SSLHashSHA1.update(&hashCtx, &clientRandom)) != 0)
goto fail;
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;
err = sslRawVerify(ctx,
ctx->peerPubKey,
dataToSign, /* plaintext */
dataToSignLen, /* plaintext length */
signature,
signatureLen);
if(err) {
sslErrorLog("SSLDecodeSignedServerKeyExchange: sslRawVerify "
"returned %d\n", (int)err);
goto fail;
}
fail:
- SSLFreeBuffer(&signedHashes, ctx);
- SSLFreeBuffer(&hashCtx, ctx);
+ SSLFreeBuffer(&signedHashes);
+ SSLFreeBuffer(&hashCtx);
return err;
}

What was he/she thinking? I don't see the need of adding a goto fail; there.

A good static analysis tool would have detected that "err" could be returned uninitialized.

That damn "duplicate line" shortcut!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.