commit 61c0d4e8b210104f6e9575421411641d9fe87086
author David Benjamin <davidben@chromium.org> Thu Mar 19 14:24:37 2015 -0400
committer Adam Langley <agl@google.com> Thu Mar 19 19:46:28 2015 +0000
tree 4106ae62c1e5cce7fa4b066c36b860dbdd0a1f94
parent c12d377e2400d0011e2269aa6df2a3193d0f60a9

Always reset sigalgslen when NULLing sigalgs.

See also upstream's 34e3edbf3a10953cb407288101fd56a629af22f9. This fixes
CVE-2015-0291. Also bubble up malloc failures in tls1_set_shared_sigalgs. Tidy
up style a bit and remove unnecessary check (it actually is unnecessary; see
https://boringssl-review.googlesource.com/4042).

Change-Id: Idfb31a90fb3e56ef6fe7701464748a5c1603f064
Reviewed-on: https://boringssl-review.googlesource.com/4047
Reviewed-by: Adam Langley <agl@google.com>

ssl/t1_lib.c

diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c
index 1cf7620..627df3f 100644
--- a/ssl/t1_lib.c
+++ b/ssl/t1_lib.c
@@ -1357,12 +1357,14 @@
   if (s->cert->peer_sigalgs) {
     OPENSSL_free(s->cert->peer_sigalgs);
     s->cert->peer_sigalgs = NULL;
+    s->cert->peer_sigalgslen = 0;
   }
 
   /* Clear any shared signature algorithms */
   if (s->cert->shared_sigalgs) {
     OPENSSL_free(s->cert->shared_sigalgs);
     s->cert->shared_sigalgs = NULL;
+    s->cert->shared_sigalgslen = 0;
   }
 
   /* Clear ECC extensions */
@@ -2399,6 +2401,7 @@
   if (c->shared_sigalgs) {
     OPENSSL_free(c->shared_sigalgs);
     c->shared_sigalgs = NULL;
+    c->shared_sigalgslen = 0;
   }
 
   /* If client use client signature algorithms if not NULL */
@@ -2449,21 +2452,12 @@
     return 1;
   }
 
-  /* Length must be even */
-  if (CBS_len(sigalgs) % 2 != 0) {
+  if (CBS_len(sigalgs) % 2 != 0 ||
+      !CBS_stow(sigalgs, &c->peer_sigalgs, &c->peer_sigalgslen) ||
+      !tls1_set_shared_sigalgs(s)) {
     return 0;
   }
 
-  /* Should never happen */
-  if (!c) {
-    return 0;
-  }
-
-  if (!CBS_stow(sigalgs, &c->peer_sigalgs, &c->peer_sigalgslen)) {
-    return 0;
-  }
-
-  tls1_set_shared_sigalgs(s);
   return 1;
 }