[PATCH] malloc: fix out-of-bounds read

[Collin Funk]

Hi,

In lib/malloc/malloc.c there is a read that occurs 1 or 2 indexes before
the first element in the buffer. The issue is this macro:

/* Use this when we want to be sure that NB is in bucket NU. */
#define RIGHT_BUCKET(nb, nu) \
        (((nb) > binsizes[(nu)-1]) && ((nb) <= binsizes[(nu)]))

Where 'binsizes' is an array like this:

static const unsigned long binsizes[NBUCKETS] = {
        32UL, 64UL, 128UL, 256UL, 512UL, 1024UL, 2048UL, 4096UL,
        ... };

The out-of-bounds read occurs in a line like this:

  /* If ok, use the same block, just marking its size as changed.  */
  if (RIGHT_BUCKET(nbytes, nunits) || RIGHT_BUCKET(nbytes, nunits-1))
    {
      ...
    }

Where 'nunits' isn't properly checked. This can easily be seen by
-fsanitize=undefined when running make check:

< malloc.c:1205:7: runtime error: index -1 out of bounds for type 'long 
unsigned int [28]'
< malloc.c:1205:39: runtime error: index -2 out of bounds for type 'long 
unsigned int [28]'
< malloc.c:1205:39: runtime error: index -1 out of bounds for type 'long 
unsigned int [28]'

I've attached a patch that silences ubsan atleast. I didn't look into
the surrounding code much so a double check would be nice. :)

Collin


>From 4863afd5260e11f05f69adc64c496f6d8bace627 Mon Sep 17 00:00:00 2001
From: Collin Funk <collin.fu...@gmail.com>
Date: Thu, 18 Jul 2024 21:45:51 -0700
Subject: [PATCH] malloc: fix out-of-bounds read

* lib/malloc/malloc.c (internal_realloc): Check value of nunits before
using RIGHT_BUCKET.
---
 lib/malloc/malloc.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lib/malloc/malloc.c b/lib/malloc/malloc.c
index 7b2c3f25..07487fa8 100644
--- a/lib/malloc/malloc.c
+++ b/lib/malloc/malloc.c
@@ -1202,7 +1202,8 @@ internal_realloc (PTR_T mem, size_t n, const char *file, int line, int flags)
   nbytes = ALLOCATED_BYTES(n);
 
   /* If ok, use the same block, just marking its size as changed.  */
-  if (RIGHT_BUCKET(nbytes, nunits) || RIGHT_BUCKET(nbytes, nunits-1))
+  if ((1 <= nunits && RIGHT_BUCKET (nbytes, nunits))
+      || (2 <= nunits && RIGHT_BUCKET (nbytes, nunits - 1)))
     {
       /* Compensate for increment above. */
       m -= 4;
-- 
2.45.2