public inbox for glibc-cvs@sourceware.org
help / color / mirror / Atom feed
From: Adhemerval Zanella <azanella@sourceware.org>
To: glibc-cvs@sourceware.org
Subject: [glibc/azanella/strncmp-fix] string: Fix OOB read on generic strncmp
Date: Thu, 23 Feb 2023 14:24:42 +0000 (GMT)	[thread overview]
Message-ID: <20230223142442.55C6F385B501@sourceware.org> (raw)

https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=52cb8571a851e954830a5f4b3c80f3b0f705ce1d

commit 52cb8571a851e954830a5f4b3c80f3b0f705ce1d
Author: Adhemerval Zanella <adhemerval.zanella@linaro.org>
Date:   Tue Feb 21 16:03:19 2023 -0300

    string: Fix OOB read on generic strncmp
    
    For unaligned case, reading ahead can only be done if parting reads
    matches the aligned input.
    
    Also extend the stratcliff tests to check such cases.
    
    Checked on x86_64-linux-gnu, i686-linux-gnu, powerpc64-linux-gnu,
    and powerpc-linux-gnu by removing the arch-specific assembly
    implementation and disabling multi-arch (it covers both LE and BE
    for 64 and 32 bits).
    
    Reviewed-by: Szabolcs Nagy <szabolcs.nagy@arm.com>

Diff:
---
 string/stratcliff.c | 17 ++++++++++++++++-
 string/strncmp.c    | 13 ++++++++++++-
 2 files changed, 28 insertions(+), 2 deletions(-)

diff --git a/string/stratcliff.c b/string/stratcliff.c
index 74d64cc03d..88ac787088 100644
--- a/string/stratcliff.c
+++ b/string/stratcliff.c
@@ -401,12 +401,27 @@ do_test (void)
 		result = 1;
 	      }
 
-	    if (STRNCMP (dest + nchars - outer, adr + middle, outer) <= 0)
+	    /* Also check for size larger than the string.  */
+	    if (STRNCMP (adr + middle, dest + nchars - outer, outer + 99) >= 0)
 	      {
 		printf ("%s 2 flunked for outer = %zu, middle = %zu, full\n",
+			STRINGIFY (STRNCMP), outer + 99, middle);
+		result = 1;
+	      }
+
+	    if (STRNCMP (dest + nchars - outer, adr + middle, outer) <= 0)
+	      {
+		printf ("%s 3 flunked for outer = %zu, middle = %zu, full\n",
 			STRINGIFY (STRNCMP), outer, middle);
 		result = 1;
 	      }
+
+	    if (STRNCMP (dest + nchars - outer, adr + middle, outer + 99) <= 0)
+	      {
+		printf ("%s 4 flunked for outer = %zu, middle = %zu, full\n",
+			STRINGIFY (STRNCMP), outer + 99, middle);
+		result = 1;
+	      }
 	  }
 
       /* strncpy/wcsncpy tests */
diff --git a/string/strncmp.c b/string/strncmp.c
index 4c8bf36bb9..751bf53d55 100644
--- a/string/strncmp.c
+++ b/string/strncmp.c
@@ -73,7 +73,11 @@ strncmp_unaligned_loop (const op_t *x1, const op_t *x2, op_t w1, uintptr_t ofs,
   uintptr_t sh_2 = sizeof(op_t) * CHAR_BIT - sh_1;
 
   op_t w2 = MERGE (w2a, sh_1, (op_t)-1, sh_2);
-  if (!has_zero (w2) && n > (sizeof (op_t) - ofs))
+
+  /* Reading ahead is wrong if w1 and w2 already differs.  */
+  op_t w1a = MERGE (w1, 0, (op_t)-1, sh_2);
+
+  if (!has_zero (w2) && w2 == w1a && n >= (sizeof (op_t) - ofs))
     {
       op_t w2b;
 
@@ -90,6 +94,13 @@ strncmp_unaligned_loop (const op_t *x1, const op_t *x2, op_t w1, uintptr_t ofs,
 	  if (has_zero (w2b) || n <= (sizeof (op_t) - ofs))
 	    break;
 	  w1 = *x1++;
+
+	  /* Reading ahead is wrong if w1 and w2 already differs.  */
+	  w2 = MERGE (w2b, sh_1, (op_t)-1, sh_2);
+	  w1a = MERGE (w1, 0, (op_t)-1, sh_2);
+	  if (w2 != w1a)
+	    return final_cmp (w1a, w2, n);
+
 	  w2a = w2b;
 	}

                 reply	other threads:[~2023-02-23 14:24 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230223142442.55C6F385B501@sourceware.org \
    --to=azanella@sourceware.org \
    --cc=glibc-cvs@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).