* [PATCH] correct buffer end pointer in IO_wdefault_doallocate (BZ #26874)
@ 2021-01-01 22:34 Martin Sebor via Libc-alpha
2021-02-04 9:47 ` Siddhesh Poyarekar
0 siblings, 1 reply; 4+ messages in thread
From: Martin Sebor via Libc-alpha @ 2021-01-01 22:34 UTC (permalink / raw
To: GNU C Library
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.
Since the bug looks like it might be exploitable I tried to create
a test case to trigger a call to _IO_wdefault_doallocate but couldn't.
No test in the test suite seems to either, so I post this patch without
one.
Martin
diff --git a/libio/wgenops.c b/libio/wgenops.c
index 0a242d93ca..153b1da8dc 100644
--- a/libio/wgenops.c
+++ b/libio/wgenops.c
@@ -379,12 +379,11 @@ libc_hidden_def (_IO_wdoallocbuf)
int
_IO_wdefault_doallocate (FILE *fp)
{
- wchar_t *buf;
-
- buf = malloc (BUFSIZ);
+ wchar_t *buf = (wchar_t *)malloc (BUFSIZ);
if (__glibc_unlikely (buf == NULL))
return EOF;
- _IO_wsetb (fp, buf, buf + BUFSIZ, 1);
+
+ _IO_wsetb (fp, buf, buf + BUFSIZ / sizeof *buf, 1);
return 1;
}
libc_hidden_def (_IO_wdefault_doallocate)
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] correct buffer end pointer in IO_wdefault_doallocate (BZ #26874)
2021-01-01 22:34 [PATCH] correct buffer end pointer in IO_wdefault_doallocate (BZ #26874) Martin Sebor via Libc-alpha
@ 2021-02-04 9:47 ` Siddhesh Poyarekar
2021-03-01 14:06 ` Siddhesh Poyarekar
0 siblings, 1 reply; 4+ messages in thread
From: Siddhesh Poyarekar @ 2021-02-04 9:47 UTC (permalink / raw
To: Martin Sebor, GNU C Library
On 1/2/21 4:04 AM, Martin Sebor via Libc-alpha wrote:
> 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.
>
> Since the bug looks like it might be exploitable I tried to create
> a test case to trigger a call to _IO_wdefault_doallocate but couldn't.
> No test in the test suite seems to either, so I post this patch without
> one.
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.
Nevertheless, this is a good cleanup, so LGTM.
Thanks,
Siddhesh
> Martin
>
> diff --git a/libio/wgenops.c b/libio/wgenops.c
> index 0a242d93ca..153b1da8dc 100644
> --- a/libio/wgenops.c
> +++ b/libio/wgenops.c
> @@ -379,12 +379,11 @@ libc_hidden_def (_IO_wdoallocbuf)
> int
> _IO_wdefault_doallocate (FILE *fp)
> {
> - wchar_t *buf;
> -
> - buf = malloc (BUFSIZ);
> + wchar_t *buf = (wchar_t *)malloc (BUFSIZ);
> if (__glibc_unlikely (buf == NULL))
> return EOF;
> - _IO_wsetb (fp, buf, buf + BUFSIZ, 1);
> +
> + _IO_wsetb (fp, buf, buf + BUFSIZ / sizeof *buf, 1);
> return 1;
> }
> libc_hidden_def (_IO_wdefault_doallocate)
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] correct buffer end pointer in IO_wdefault_doallocate (BZ #26874)
2021-02-04 9:47 ` Siddhesh Poyarekar
@ 2021-03-01 14:06 ` Siddhesh Poyarekar
2021-03-01 16:34 ` Martin Sebor via Libc-alpha
0 siblings, 1 reply; 4+ messages in thread
From: Siddhesh Poyarekar @ 2021-03-01 14:06 UTC (permalink / raw
To: Martin Sebor, GNU C Library
On 2/4/21 3:17 PM, Siddhesh Poyarekar wrote:
> On 1/2/21 4:04 AM, Martin Sebor via Libc-alpha wrote:
>> 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.
>>
>> Since the bug looks like it might be exploitable I tried to create
>> a test case to trigger a call to _IO_wdefault_doallocate but couldn't.
>> No test in the test suite seems to either, so I post this patch without
>> one.
>
> 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.
>
> Nevertheless, this is a good cleanup, so LGTM.
Martin, I've pushed this patch for you.
Siddhesh
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] correct buffer end pointer in IO_wdefault_doallocate (BZ #26874)
2021-03-01 14:06 ` Siddhesh Poyarekar
@ 2021-03-01 16:34 ` Martin Sebor via Libc-alpha
0 siblings, 0 replies; 4+ messages in thread
From: Martin Sebor via Libc-alpha @ 2021-03-01 16:34 UTC (permalink / raw
To: Siddhesh Poyarekar, GNU C Library
On 3/1/21 7:06 AM, Siddhesh Poyarekar wrote:
> On 2/4/21 3:17 PM, Siddhesh Poyarekar wrote:
>> On 1/2/21 4:04 AM, Martin Sebor via Libc-alpha wrote:
>>> 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.
>>>
>>> Since the bug looks like it might be exploitable I tried to create
>>> a test case to trigger a call to _IO_wdefault_doallocate but couldn't.
>>> No test in the test suite seems to either, so I post this patch without
>>> one.
>>
>> 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.
>>
>> Nevertheless, this is a good cleanup, so LGTM.
>
> Martin, I've pushed this patch for you.
Thanks! (And sorry for not getting to it myself. I'm behind on
a number of fronts.)
Martin
>
> Siddhesh
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-03-01 16:34 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-01-01 22:34 [PATCH] correct buffer end pointer in IO_wdefault_doallocate (BZ #26874) Martin Sebor via Libc-alpha
2021-02-04 9:47 ` Siddhesh Poyarekar
2021-03-01 14:06 ` Siddhesh Poyarekar
2021-03-01 16:34 ` Martin Sebor via Libc-alpha
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).