[Bug build/26874] New: -Warray-bounds in _IO_wdefault_doallocate

[Bug build/26874] New: -Warray-bounds in _IO_wdefault_doallocate

[msebor at gmail dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=26874

            Bug ID: 26874
           Summary: -Warray-bounds in _IO_wdefault_doallocate
           Product: glibc
           Version: unspecified
            Status: UNCONFIRMED
          Severity: normal
          Priority: P2
         Component: build
          Assignee: unassigned at sourceware dot org
          Reporter: msebor at gmail dot com
                CC: carlos at redhat dot com
  Target Milestone: ---

A recent Glibc build with an experimental version GCC 11 with some
-Warray-bounds enhancements shows the warning below:

wgenops.c: In function ‘_IO_wdefault_doallocate’:
wgenops.c:387:3: warning: array subscript 8192 is outside array bounds of
‘wchar_t[2048]’ {aka ‘int[2048]’} [-Warray-bounds]
  387 |   _IO_wsetb (fp, buf, buf + BUFSIZ, 1);
      |   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
wgenops.c:384:9: note: at offset 32768 into source object of size 8192
allocated by ‘malloc’
  384 |   buf = malloc (BUFSIZ);
      |         ^~~~~~~~~~~~~~~

The warning looks justified to me.  The function allocates BUFSIZ bytes but
then increments the wchar_t* pointer to it by the same number, failing to take
into account the scaling by sizeof (wchar_t):

int
_IO_wdefault_doallocate (FILE *fp)
{
  wchar_t *buf;

  buf = malloc (BUFSIZ);
  if (__glibc_unlikely (buf == NULL))
    return EOF;
  _IO_wsetb (fp, buf, buf + BUFSIZ, 1);
  return 1;
}

I'd say either the malloc argument should be scaled up or the pointer increment
scaled down.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug build/26874] -Warray-bounds in _IO_wdefault_doallocate

[msebor at gmail dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=26874

--- Comment #1 from Martin Sebor <msebor at gmail dot com> ---
Patch: https://sourceware.org/pipermail/libc-alpha/2021-January/121244.html

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug build/26874] -Warray-bounds in _IO_wdefault_doallocate

[siddhesh at sourceware dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=26874

Siddhesh Poyarekar <siddhesh at sourceware dot org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |siddhesh at sourceware dot org
              Flags|                            |security-
         Resolution|---                         |FIXED
             Status|UNCONFIRMED                 |RESOLVED
   Target Milestone|---                         |2.34

--- Comment #2 from Siddhesh Poyarekar <siddhesh at sourceware dot org> ---
Fixed in main:

https://sourceware.org/git/?p=glibc.git;a=commit;h=764e9a0334350f52ab6953bef1db97f9b2e89ca5

commit 764e9a0334350f52ab6953bef1db97f9b2e89ca5 (HEAD -> master, origin/master,
origin/HEAD)
Author: Martin Sebor <msebor@gmail.com>
Date:   Mon Mar 1 10:35:39 2021 +0530

    Correct buffer end pointer in IO_wdefault_doallocate (BZ #26874)

    An experimental build of GCC 11 with an enhanced -Warray-bounds
    reports a bug in IO_wdefault_doallocate where the function forms
    an invalid past-the-end pointer to an allocated wchar_t buffer
    by failingf to consider the scaling by sizeof (wchar_t).

    The fix path below corrects this problem.  It keeps the buffer
    size the same as opposed to increasing it according to what other
    code like it does.

Marking this security- with the following rationale I posted on list:

I spent some time following those vtable setups and I don't think
_IO_wdefault_doallocate gets called at all.  The only instances it is set as
the doallocate callback are in strops, wmemstream and in vswprintf.  In all
those cases, the backing buffer is either user supplied or is sized without
using doallocate (as in the case of strops overflow).

The only time libio needs to allocate buffers in the wide context is in
wfileops when it's actually using the underlying buffer.  In that case however,
the doallocate callback is different.

-- 
You are receiving this mail because:
You are on the CC list for the bug.