diff options
author | Arjun Shankar <arjun@redhat.com> | 2024-01-15 17:44:43 +0100 |
---|---|---|
committer | Arjun Shankar <arjun@redhat.com> | 2024-01-30 15:53:37 +0100 |
commit | 6bd0e4efcc78f3c0115e5ea9739a1642807450da (patch) | |
tree | 18ec2375433978e7a6aef01108958524f539a8d6 /misc/syslog.c | |
parent | 8aeec0eb5a18f9614d18156f9d6092b3525b818c (diff) | |
download | glibc-6bd0e4efcc78f3c0115e5ea9739a1642807450da.tar.gz glibc-6bd0e4efcc78f3c0115e5ea9739a1642807450da.tar.xz glibc-6bd0e4efcc78f3c0115e5ea9739a1642807450da.zip |
syslog: Fix heap buffer overflow in __vsyslog_internal (CVE-2023-6246)
__vsyslog_internal did not handle a case where printing a SYSLOG_HEADER containing a long program name failed to update the required buffer size, leading to the allocation and overflow of a too-small buffer on the heap. This commit fixes that. It also adds a new regression test that uses glibc.malloc.check. Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org> Reviewed-by: Carlos O'Donell <carlos@redhat.com> Tested-by: Carlos O'Donell <carlos@redhat.com>
Diffstat (limited to 'misc/syslog.c')
-rw-r--r-- | misc/syslog.c | 50 |
1 files changed, 36 insertions, 14 deletions
diff --git a/misc/syslog.c b/misc/syslog.c index 1b8cb722c5..814d224a1e 100644 --- a/misc/syslog.c +++ b/misc/syslog.c @@ -124,8 +124,9 @@ __vsyslog_internal (int pri, const char *fmt, va_list ap, { /* Try to use a static buffer as an optimization. */ char bufs[1024]; - char *buf = NULL; - size_t bufsize = 0; + char *buf = bufs; + size_t bufsize; + int msgoff; int saved_errno = errno; @@ -177,29 +178,50 @@ __vsyslog_internal (int pri, const char *fmt, va_list ap, #define SYSLOG_HEADER_WITHOUT_TS(__pri, __msgoff) \ "<%d>: %n", __pri, __msgoff - int l; + int l, vl; if (has_ts) l = __snprintf (bufs, sizeof bufs, SYSLOG_HEADER (pri, timestamp, &msgoff, pid)); else l = __snprintf (bufs, sizeof bufs, SYSLOG_HEADER_WITHOUT_TS (pri, &msgoff)); + + char *pos; + size_t len; + if (0 <= l && l < sizeof bufs) { - va_list apc; - va_copy (apc, ap); + /* At this point, there is still a chance that we can print the + remaining part of the log into bufs and use that. */ + pos = bufs + l; + len = sizeof (bufs) - l; + } + else + { + buf = NULL; + /* We already know that bufs is too small to use for this log message. + The next vsnprintf into bufs is used only to calculate the total + required buffer length. We will discard bufs contents and allocate + an appropriately sized buffer later instead. */ + pos = bufs; + len = sizeof (bufs); + } - /* Restore errno for %m format. */ - __set_errno (saved_errno); + { + va_list apc; + va_copy (apc, ap); - int vl = __vsnprintf_internal (bufs + l, sizeof bufs - l, fmt, apc, - mode_flags); - if (0 <= vl && vl < sizeof bufs - l) - buf = bufs; - bufsize = l + vl; + /* Restore errno for %m format. */ + __set_errno (saved_errno); - va_end (apc); - } + vl = __vsnprintf_internal (pos, len, fmt, apc, mode_flags); + + if (!(0 <= vl && vl < len)) + buf = NULL; + + bufsize = l + vl; + va_end (apc); + } if (buf == NULL) { |