glibc 2.36: syslog() with LOG_PERROR and a message > 1024 ends up reading invalid memory

glibc 2.36: syslog() with LOG_PERROR and a message > 1024 ends up reading invalid memory

[Aleksander Morgado]

[-- Attachment #1: Type: text/plain, Size: 2280 bytes --]

Hey all,

I'm debugging memory issues in ModemManager running it under valgrind
and I believe I've hit a bug in the syslog() implementation in glibc
2.36 when using LOG_PERROR.

The call triggering the invalid error is the __dprintf() call in line 230:
https://elixir.bootlin.com/glibc/glibc-2.36/source/misc/syslog.c#L230

  /* Output to stderr if requested. */
  if (LogStat & LOG_PERROR)
    __dprintf (STDERR_FILENO, "%s%s", buf + msgoff,
      "\n" + (buf[bufsize - 1] == '\n'));

If I'm reading the code right, I believe that bufsize is only set to a
value != 0 if the logic ends up using the static "bufs" buffer. If the
logic needs to allocate memory for a longer buffer, bufsize is never
initialized, so the __dprintf call above tries to access buf[-1].

Valgrind reports the problem as:

 valgrind ./test-syslog-valgrind-error
==20815== Memcheck, a memory error detector
==20815== Copyright (C) 2002-2022, and GNU GPL'd, by Julian Seward et al.
==20815== Using Valgrind-3.19.0 and LibVEX; rerun with -h for copyright info
==20815== Command: ./test-syslog-valgrind-error
==20815==
==20815== Invalid read of size 1
==20815==    at 0x4985E58: __vsyslog_internal (syslog.c:230)
==20815==    by 0x4986299: syslog (syslog.c:90)
==20815==    by 0x1091AB: main (in /home/aleksander/test-syslog-valgrind-error)
==20815==  Address 0x4a71baf is 1 bytes before a block of size 29 alloc'd
==20815==    at 0x4841888: malloc (in
/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==20815==    by 0x4985DCC: __vsyslog_internal (syslog.c:206)
==20815==    by 0x4986299: syslog (syslog.c:90)
==20815==    by 0x1091AB: main (in /home/aleksander/test-syslog-valgrind-error)
==20815==
==20815== Conditional jump or move depends on uninitialised value(s)
==20815==    at 0x4847D09: strlen (in
/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==20815==    by 0x48E15C7: __vfprintf_internal (vfprintf-process-arg.c:397)
==20815==    by 0x49016F9: __vdprintf_internal (iovdprintf.c:54)
==20815==    by 0x48D4D89: dprintf (dprintf.c:30)
==20815==    by 0x4985E82: __vsyslog_internal (syslog.c:230)
==20815==    by 0x4986299: syslog (syslog.c:90)
==20815==    by 0x1091AB: main (in /home/aleksander/test-syslog-valgrind-error)
==20815==

Attached is a simple tester.

Cheers

-- 
Aleksander

[-- Attachment #2: test-syslog-valgrind-error.c --]
[-- Type: text/x-csrc, Size: 2921 bytes --]


#include <stdio.h>
#include <syslog.h>

int main (int argc, const char *argv[])
{
  const char *some_very_long_message = "Lorem ipsum dolor sit amet, consectetur adipiscing elit. Nulla gravida sapien metus, in sagittis ipsum pellentesque ut. In dui lectus, elementum ut lacus et, mattis ullamcorper nulla. Cras vel arcu laoreet, fringilla lacus sit amet, scelerisque nisl. Suspendisse nec massa eu erat commodo mollis. Curabitur imperdiet velit id lectus laoreet auctor. Sed in enim volutpat, vulputate ipsum quis, tristique nulla. Vestibulum vitae condimentum metus, nec commodo lacus. Aliquam erat volutpat. Nunc fringilla justo at feugiat elementum. Aliquam eget nisl vel arcu molestie placerat ut non lectus. Vivamus scelerisque condimentum felis ut hendrerit. Pellentesque sit amet dui eu erat lacinia gravida nec vitae nisl. Suspendisse rhoncus sagittis lacus, pharetra porttitor libero laoreet eu. Proin scelerisque luctus blandit. Maecenas non odio sapien. Vivamus id euismod lorem, at maximus nisi. Maecenas consectetur et felis at tempus. Etiam ac laoreet sem, vitae dignissim nulla. Nulla eu pretium nulla. In nec auctor nisl. Fusce luctus vel dolor id tempus. Nunc varius nunc eros, eget mattis sapien efficitur at. Duis dolor est, vestibulum eu interdum a, interdum id augue. Donec hendrerit, mi non laoreet placerat, nunc turpis scelerisque dui, eu pulvinar dui dui facilisis diam. Curabitur sapien risus, varius in neque eget, molestie rutrum dui. Etiam dolor nulla, sollicitudin nec mauris in, blandit pretium nulla. Orci varius natoque penatibus et magnis dis parturient montes, nascetur ridiculus mus. Donec lacinia mollis rutrum. Morbi aliquet tempus odio, ac euismod mi fermentum a. Duis ut facilisis tortor. Curabitur egestas nisi quis pulvinar porta. Sed consectetur interdum metus, eleifend condimentum massa congue at. Etiam vel rhoncus enim. Nullam bibendum velit ut ultricies aliquam. Maecenas in varius elit, nec sollicitudin lectus. Nulla eleifend scelerisque nulla, eu vehicula tortor vulputate vitae. In consequat vitae ipsum in sollicitudin. Nam rutrum libero mauris, nec iaculis lectus lobortis vel. Donec eget tempus nibh. Etiam egestas ultrices tortor, ac condimentum tellus ultricies in. Nulla commodo hendrerit metus nec feugiat. Donec libero tortor, posuere sit amet metus malesuada, commodo vulputate ipsum. Nam a auctor augue. Sed vel libero dui. Donec scelerisque dignissim risus, eget aliquet arcu vestibulum nec. Aliquam nec arcu vel felis sollicitudin lacinia. Curabitur eget purus nibh. Phasellus rutrum vulputate nunc, sit amet ullamcorper sem congue eu. Nam interdum nibh turpis, vehicula sagittis quam dictum vel. Curabitur dolor sem, pulvinar a velit ac, ultrices tincidunt felis. Quisque vitae mollis ipsum. Morbi quis tortor a metus iaculis elementum.";

  openlog ("MyTest", LOG_PERROR, LOG_DAEMON);
  syslog (LOG_DEBUG, "%s", some_very_long_message);
  closelog ();
}

Re: glibc 2.36: syslog() with LOG_PERROR and a message > 1024 ends up reading invalid memory

[Adhemerval Zanella Netto]

On 26/08/22 17:12, Aleksander Morgado via Libc-alpha wrote:
> Hey all,
> 
> I'm debugging memory issues in ModemManager running it under valgrind
> and I believe I've hit a bug in the syslog() implementation in glibc
> 2.36 when using LOG_PERROR.
> 
> The call triggering the invalid error is the __dprintf() call in line 230:
> https://elixir.bootlin.com/glibc/glibc-2.36/source/misc/syslog.c#L230
> 
>   /* Output to stderr if requested. */
>   if (LogStat & LOG_PERROR)
>     __dprintf (STDERR_FILENO, "%s%s", buf + msgoff,
>       "\n" + (buf[bufsize - 1] == '\n'));
> 
> If I'm reading the code right, I believe that bufsize is only set to a
> value != 0 if the logic ends up using the static "bufs" buffer. If the
> logic needs to allocate memory for a longer buffer, bufsize is never
> initialized, so the __dprintf call above tries to access buf[-1].
> 
> Valgrind reports the problem as:
> 
>  valgrind ./test-syslog-valgrind-error
> ==20815== Memcheck, a memory error detector
> ==20815== Copyright (C) 2002-2022, and GNU GPL'd, by Julian Seward et al.
> ==20815== Using Valgrind-3.19.0 and LibVEX; rerun with -h for copyright info
> ==20815== Command: ./test-syslog-valgrind-error
> ==20815==
> ==20815== Invalid read of size 1
> ==20815==    at 0x4985E58: __vsyslog_internal (syslog.c:230)
> ==20815==    by 0x4986299: syslog (syslog.c:90)
> ==20815==    by 0x1091AB: main (in /home/aleksander/test-syslog-valgrind-error)
> ==20815==  Address 0x4a71baf is 1 bytes before a block of size 29 alloc'd
> ==20815==    at 0x4841888: malloc (in
> /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
> ==20815==    by 0x4985DCC: __vsyslog_internal (syslog.c:206)
> ==20815==    by 0x4986299: syslog (syslog.c:90)
> ==20815==    by 0x1091AB: main (in /home/aleksander/test-syslog-valgrind-error)
> ==20815==
> ==20815== Conditional jump or move depends on uninitialised value(s)
> ==20815==    at 0x4847D09: strlen (in
> /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
> ==20815==    by 0x48E15C7: __vfprintf_internal (vfprintf-process-arg.c:397)
> ==20815==    by 0x49016F9: __vdprintf_internal (iovdprintf.c:54)
> ==20815==    by 0x48D4D89: dprintf (dprintf.c:30)
> ==20815==    by 0x4985E82: __vsyslog_internal (syslog.c:230)
> ==20815==    by 0x4986299: syslog (syslog.c:90)
> ==20815==    by 0x1091AB: main (in /home/aleksander/test-syslog-valgrind-error)
> ==20815==
> 
> Attached is a simple tester.
> 
> Cheers
> 

Thanks, in fact this is not the only issue, the message is not log for large buffers.
I will take of it, I should have added tests for large buffers on tst-syslog as well.