From c6722cd6e0da9c7f00566cbe4bf7eb3dca107cf6 Mon Sep 17 00:00:00 2001 From: David Benjamin Date: Mon, 24 Oct 2016 20:13:20 -0400 Subject: [PATCH] Check SSL_MODE_ACCEPT_MOVING_WRITE_BUFFER before touching wpend_buf. SSL_write has messy semantics around retries. As a sanity-check, it does pointer and length checks and requires the original and retry SSL_write pass the same buffer pointer. In some cases, buffer addresses may change but still include the original data as a prefix on the retry. Callers then set SSL_MODE_ACCEPT_MOVING_WRITE_BUFFER to skip the pointer check. But, in that case, the pointer may have been freed so doing a comparison is undefined behavior. Short-circuiting the pointer equality check avoids this problem. Change-Id: I76cb8f7d45533504cd95287bc53897ca636af51d Reviewed-on: https://boringssl-review.googlesource.com/11760 CQ-Verified: CQ bot account: commit-bot@chromium.org Reviewed-by: Steven Valdez Reviewed-by: David Benjamin Commit-Queue: David Benjamin --- ssl/s3_pkt.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ssl/s3_pkt.c b/ssl/s3_pkt.c index fda9a251..3cf18b7a 100644 --- a/ssl/s3_pkt.c +++ b/ssl/s3_pkt.c @@ -245,8 +245,8 @@ int ssl3_write_bytes(SSL *ssl, int type, const void *buf_, int len) { static int ssl3_write_pending(SSL *ssl, int type, const uint8_t *buf, unsigned int len) { if (ssl->s3->wpend_tot > (int)len || - (ssl->s3->wpend_buf != buf && - !(ssl->mode & SSL_MODE_ACCEPT_MOVING_WRITE_BUFFER)) || + (!(ssl->mode & SSL_MODE_ACCEPT_MOVING_WRITE_BUFFER) && + ssl->s3->wpend_buf != buf) || ssl->s3->wpend_type != type) { OPENSSL_PUT_ERROR(SSL, SSL_R_BAD_WRITE_RETRY); return -1;