[bug #64551] Possible null pointer dereference on the function get_buffer

[HaoxinTu]

URL:
  <https://savannah.gnu.org/bugs/?64551>

                 Summary: Possible null pointer dereference on the function
get_buffer
                   Group: make
               Submitter: haoxin
               Submitted: Sun 13 Aug 2023 09:12:11 AM UTC
                Severity: 3 - Normal
              Item Group: Bug
                  Status: None
                 Privacy: Public
             Assigned to: None
             Open/Closed: Open
         Discussion Lock: Any
       Component Version: 4.2
        Operating System: None
           Fixed Release: None
           Triage Status: None


    _______________________________________________________

Follow-up Comments:


-------------------------------------------------------
Date: Sun 13 Aug 2023 09:12:11 AM UTC By: HaoxinTu <haoxin>
Hi, 

We have developed a new tool built on top of KLEE (http://klee.github.io/) to
automatically test make-4.2 and found there might be a possible null pointer
dereference in the function get_buffer in output.c:605. Here is the stack info
when the error occurs:

Stack: 
        #000010267 in get_buffer (need=69) at ../make-4.2/output.c:605
        #100010161 in fatal (flocp=0, len=0, fmt=93825073769664) at
../make-4.2/output.c:683
        #200012801 in xrealloc (ptr=0, size=138) at ../make-4.2/misc.c:252
        #300010258 in get_buffer (need=69) at ../make-4.2/output.c:602
        #400010161 in fatal (flocp=0, len=0, fmt=93825073769664) at
../make-4.2/output.c:683
        #500021390 in xmalloc (size=200) at ../make-4.2/misc.c:223
        #600054772 in eval_makefile (filename=93825076010318, flags=0) at
../make-4.2/read.c:432
        #700066966 in read_all_makefiles (makefiles=93825076779920) at
../make-4.2/read.c:218
        #800062387 in __klee_posix_wrapped_main (argc=4, argv=93825076037408) at
../make-4.2/main.c:1970
        #900008309 in __user_main (=7, =93825073087424, =93825073087488) at
runtime/POSIX/klee_init_env.c:252
        #1000001882 in __uClibc_main (=93825022291208, =7, =93825073087424, =0, 
=0,
=0, =0) at libc/misc/internals/__uClibc_main.c:401
        #1100002047 in main (=7, =93825073087424)

Here is the main code involved in the error:
```
void * xrealloc (void *ptr, unsigned int size) { 
  void *result;
  result = ptr ? realloc (ptr, size) : malloc (size);  
  if (result == 0) 
    OUT_OF_MEM(); 
  return result; 
}
void fatal (const floc *flocp, size_t len, ...) { 
  char * p = get_buffer (len);  
  ...
  die (MAKE_FAILURE); 
}
static struct fmtstring {char *buffer; size_t size;} fmtbuf = {NULL, 0};
static char * get_buffer (size_t need) { 
  if (need > fmtbuf.size) { 
      fmtbuf.size += need * 2; 
      fmtbuf.buffer = xrealloc (fmtbuf.buffer, fmtbuf.size);
  }
  fmtbuf.buffer[need-1] = '\0'; // null pointer dereference
  return fmtbuf.buffer; 
}
```

The error happens due to the lack of handling of consecutive NULL pointer
returns. Normally, a NULL pointer checking is conducted after allocating the
memory in function `xrealloc`. If the return value from `realloc` or `malloc`
equals 0, the program will be terminated using the function OUT_OF_MEM as
expected. It seems ok so far. However, a memory error might happen when
another `xrealloc` is called and it returns NULL again. Inside the function
`fatal`, it first allocates a buffer via function `get_buffer` based on the
required length len and terminates the execution via function `die` normally.
In the function `get_buffer`, it first checks whether the needed size need is
larger than the defined buffer size `fmtbuf.size`. If the condition is
satisfied, the buffer `fmtbuf.buffer` will be updated through another
`xrealloc`. The error happens if the second `xrealloc` returns 0. If this
occurs, the value stored in `fmtbuf.buffer` is 0. Since the value of
`fmtbuf.size` was doubled and it was defined as global scope, the condition of
if-statement will be false, and the `fmtbuf.buffer` will still keep the
original NULL. Once this situation occurs, the dereferencing of the pointer
`fmtbuf.buffer[need-1]` will lead to the null pointer dereference.

We only tested the make-4.2 version but the latest versions may have the same
potential issue after we checked the code. Can you please take a look and
check if this is a valid issue? 

Adding a simple checking of the pointer `fmtbuf.buffer` after the if branch in
the function `get_buffer` should avoid the potential issue if it is indeed an
issue. Thanks.

Best,
Haoxin







    _______________________________________________________

Reply to this item at:

  <https://savannah.gnu.org/bugs/?64551>

_______________________________________________
Message sent via Savannah
https://savannah.gnu.org/