diff mbox series

stdlib: Fix __libc_message_impl iovec size

Message ID 20250430194401.818624-1-adhemerval.zanella@linaro.org
State New
Headers show
Series stdlib: Fix __libc_message_impl iovec size | expand

Commit Message

Adhemerval Zanella Netto April 30, 2025, 7:43 p.m. UTC
The iovec size should account for all substrings between each conversion
specification.  For the format:

  "abc %s efg"

The list of substrings are:

  ["abc ", arg, " efg]

which is 2 times the number of maximum arguments *plus* one.

This issue triggered 'out of bounds' errors by stdlib/tst-bz20544 when
glibc is built with experimental UBSAN support [1].

Checked on x86_64-linux-gnu.

[1] https://sourceware.org/git/?p=glibc.git;a=shortlog;h=refs/heads/azanella/ubsan-undef
---
 sysdeps/posix/libc_fatal.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Florian Weimer May 1, 2025, 10:57 a.m. UTC | #1
* Adhemerval Zanella:

> The iovec size should account for all substrings between each conversion
> specification.  For the format:
>
>   "abc %s efg"
>
> The list of substrings are:
>
>   ["abc ", arg, " efg]
>
> which is 2 times the number of maximum arguments *plus* one.
>
> This issue triggered 'out of bounds' errors by stdlib/tst-bz20544 when
> glibc is built with experimental UBSAN support [1].
>
> Checked on x86_64-linux-gnu.
>
> [1] https://sourceware.org/git/?p=glibc.git;a=shortlog;h=refs/heads/azanella/ubsan-undef

Should this be treated as a security vulnerability?
Adhemerval Zanella Netto May 2, 2025, 12:49 p.m. UTC | #2
On 01/05/25 07:57, Florian Weimer wrote:
> * Adhemerval Zanella:
> 
>> The iovec size should account for all substrings between each conversion
>> specification.  For the format:
>>
>>   "abc %s efg"
>>
>> The list of substrings are:
>>
>>   ["abc ", arg, " efg]
>>
>> which is 2 times the number of maximum arguments *plus* one.
>>
>> This issue triggered 'out of bounds' errors by stdlib/tst-bz20544 when
>> glibc is built with experimental UBSAN support [1].
>>
>> Checked on x86_64-linux-gnu.
>>
>> [1] https://sourceware.org/git/?p=glibc.git;a=shortlog;h=refs/heads/azanella/ubsan-undef
> 
> Should this be treated as a security vulnerability?

I am not sure, although it is a write primitive it would always write
the same pattern (the pointer to the last substring, "\n") and the
function never returns. But I am not well versed in offensive programming,
so it might be something I am not seeing it.
Carlos O'Donell May 5, 2025, 1:46 p.m. UTC | #3
On 5/2/25 8:49 AM, Adhemerval Zanella Netto wrote:
> 
> 
> On 01/05/25 07:57, Florian Weimer wrote:
>> * Adhemerval Zanella:
>>
>>> The iovec size should account for all substrings between each conversion
>>> specification.  For the format:
>>>
>>>    "abc %s efg"
>>>
>>> The list of substrings are:
>>>
>>>    ["abc ", arg, " efg]
>>>
>>> which is 2 times the number of maximum arguments *plus* one.
>>>
>>> This issue triggered 'out of bounds' errors by stdlib/tst-bz20544 when
>>> glibc is built with experimental UBSAN support [1].
>>>
>>> Checked on x86_64-linux-gnu.
>>>
>>> [1] https://sourceware.org/git/?p=glibc.git;a=shortlog;h=refs/heads/azanella/ubsan-undef
>>
>> Should this be treated as a security vulnerability?
> 
> I am not sure, although it is a write primitive it would always write
> the same pattern (the pointer to the last substring, "\n") and the
> function never returns. But I am not well versed in offensive programming,
> so it might be something I am not seeing it.
> 

Please open a bug report for this issue to track our decision, and we should
mark it security- IMO.

If the data triggering the overflow cannot come from an untrusted source,
(though which assert triggers might be controllable) then we should not treat
this as a security issue, but a normal bug.

Since the function never returns you also never free the struct iovec, which
is different from other functions that might have done such operations.
diff mbox series

Patch

diff --git a/sysdeps/posix/libc_fatal.c b/sysdeps/posix/libc_fatal.c
index d90cc6c681..25ef20cfc1 100644
--- a/sysdeps/posix/libc_fatal.c
+++ b/sysdeps/posix/libc_fatal.c
@@ -61,7 +61,7 @@  __libc_message_impl (const char *fmt, ...)
   if (fd == -1)
     fd = STDERR_FILENO;
 
-  struct iovec iov[LIBC_MESSAGE_MAX_ARGS * 2 - 1];
+  struct iovec iov[LIBC_MESSAGE_MAX_ARGS * 2 + 1];
   int iovcnt = 0;
   ssize_t total = 0;