From f99f2448bd06ffa96575397d37a5caaebf3018d4 Mon Sep 17 00:00:00 2001 From: Adam Langley Date: Sun, 2 Oct 2016 09:53:38 -0700 Subject: [PATCH] Return immediately if a cipher command is invalid. Breaking from inside the inner loop doesn't do what the code wants. Instead the outer loop will continue running and it's possible for it to read off the end of the buffer. (Found with libFuzzer.) Next change will update the other abort points in this code to match. Change-Id: I006dca0cd4c31db1c4b5e84b996fe24b2f1e6c13 Reviewed-on: https://boringssl-review.googlesource.com/11421 Reviewed-by: David Benjamin Commit-Queue: David Benjamin CQ-Verified: CQ bot account: commit-bot@chromium.org --- ssl/ssl_cipher.c | 4 +--- ssl/ssl_test.cc | 2 ++ 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/ssl/ssl_cipher.c b/ssl/ssl_cipher.c index 55070e9b..8c76419c 100644 --- a/ssl/ssl_cipher.c +++ b/ssl/ssl_cipher.c @@ -1277,9 +1277,7 @@ static int ssl_cipher_process_rulestr(const SSL_PROTOCOL_METHOD *ssl_method, /* We hit something we cannot deal with, it is no command or separator * nor alphanumeric, so we call this an error. */ OPENSSL_PUT_ERROR(SSL, SSL_R_INVALID_COMMAND); - retval = in_group = 0; - l++; - break; + return 0; } if (rule == CIPHER_SPECIAL) { diff --git a/ssl/ssl_test.cc b/ssl/ssl_test.cc index 94551177..be6443cd 100644 --- a/ssl/ssl_test.cc +++ b/ssl/ssl_test.cc @@ -252,6 +252,8 @@ static const char *kBadRules[] = { "[ECDHE-RSA-CHACHA20-POLY1305|ECDHE-RSA-AES128-GCM-SHA256]:!FOO", "[ECDHE-RSA-CHACHA20-POLY1305|ECDHE-RSA-AES128-GCM-SHA256]:-FOO", "[ECDHE-RSA-CHACHA20-POLY1305|ECDHE-RSA-AES128-GCM-SHA256]:@STRENGTH", + // Opcode supplied, but missing selector. + "+", }; static const char *kMustNotIncludeNull[] = {