Package: emacs;
Reported by: Pip Cet <pipcet <at> protonmail.com>
Date: Mon, 27 Jan 2025 19:14:01 UTC
Severity: normal
Done: Paul Eggert <eggert <at> cs.ucla.edu>
Bug is archived. No further changes may be made.
View this message in rfc822 format
From: Pip Cet <pipcet <at> protonmail.com> To: 75900 <at> debbugs.gnu.org Cc: Paul Eggert <eggert <at> cs.ucla.edu> Subject: bug#75900: doprnt.c buffer overflow Date: Mon, 27 Jan 2025 20:36:28 +0000
"Pip Cet" <pipcet <at> protonmail.com> writes:
> doprnt sometimes overflows its buffer, causing stack smashing and (if
> we're lucky) a glibc abort when make_formatted_string calls it:
>
> (gdb) p make_formatted_string
> ("01234567890123456789012345678901234567890123456789012345678901234567890123456789)0123456789)01234567890123456789012345678901234567890123456789")
> [Thread 0x7fffda9ff6c0 (LWP 20144) exited]
> *** stack smashing detected ***: terminated
>
> The reason is that in doprnt, the variable bufsize indicates the number
> of remaining bytes in the buf, and bufptr points to the current byte in
> the buf, but sometimes a byte is written and bufsize is not updated:
>
> else if (! LEADING_CODE_P (fmtchar))
> {
> if (EQ (quoting_style, Qstraight) && fmtchar == '`')
> fmtchar = '\'';
>
> *bufptr++ = fmtchar;
> continue;
> }
>
> and
>
> {
> do
> *bufptr++ = *src++;
> while (--srclen != 0);
> }
>
> do this. In the former case, we must update bufsize as it will be used
> again. In the latter case, it's sufficient to set it to 0 as this is
> the last successful printing operation in this call.
>
> A related issue is that if a multibyte character produced by a %s format
> option would overflow the buffer, doprnt returns the buffer size minus
> one, as expected, but hasn't actually modified all of the buffer data:
> the final bytes will refer to uninitialized stack data.
>
> For example, on my system, where FRAME_MESSAGE_BUF_SIZE (f) is 832, this
> call:
>
> (gdb) p message ("%s%s", "xxxx", SDATA(Fmake_string (0x8aa, 0x10002, Qt)))
>
> will print the string
>
> "xxxx䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀䀀^@Z"
>
> where the last characters are an ASCII NUL followed by a 'Z' (which I
> put in the buffer after it was SAFE_ALLOCA'd). The 'Z' should have been
> overwritten, in which case message would still have printed two
> unintended NUL characters.
>
> I intend to fix the first two bugs, and fix message in a somewhat ugly
> way for the other two.
>
> Patches to follow once there is a bug number.
The first patch fixes doprnt by avoiding buffer overflows and clearing
the rest of the buffer even when the first character not to fit there
was multibyte.
From 49e1cc9ae476663cb1ddf6106f7a6f64cd558208 Mon Sep 17 00:00:00 2001
From: Pip Cet <pipcet <at> protonmail.com>
Subject: [PATCH 1/2] Fix buffer overflows in doprnt (bug#75900)
* src/doprnt.c (doprnt): Clear rest of buffer on multibyte overflow.
Always decrement bufsize when writing a byte.
---
src/doprnt.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/src/doprnt.c b/src/doprnt.c
index 335223f972a..d4b2d95fdc1 100644
--- a/src/doprnt.c
+++ b/src/doprnt.c
@@ -446,7 +446,8 @@ doprnt (char *buffer, ptrdiff_t bufsize, const char *format,
while (tem != 0);
memcpy (bufptr, string, tem);
- bufptr[tem] = 0;
+ while (tem < bufsize)
+ bufptr[tem++] = 0;
/* Trigger exit from the loop, but make sure we
return to the caller a value which will indicate
that the buffer was too small. */
@@ -498,6 +499,7 @@ doprnt (char *buffer, ptrdiff_t bufsize, const char *format,
fmtchar = '\'';
*bufptr++ = fmtchar;
+ bufsize--;
continue;
}
else
@@ -523,7 +525,10 @@ doprnt (char *buffer, ptrdiff_t bufsize, const char *format,
else
{
do
- *bufptr++ = *src++;
+ {
+ *bufptr++ = *src++;
+ bufsize--;
+ }
while (--srclen != 0);
}
}
--
2.47.1
The second patch fixes interactive vmessage to deal with doprnt's
peculiar behavior when a multibyte character doesn't fit in the buffer:
doprnt returns bufsize - 1, even though there are fewer valid bytes in
the buffer.
Note that the previous allocation of the stack buffer was overly
generous, except in the very unlikely case that FRAME_MESSAGE_BUF_SIZE
returns 0.
From c62aed0270390d19bba251f356f406963c89baf5 Mon Sep 17 00:00:00 2001
From: Pip Cet <pipcet <at> protonmail.com>
Subject: [PATCH 2/2] Avoid printing NUL characters in 'message' (bug#75900)
* src/xdisp.c (vmessage): Increase buffer size to fit an extra
multibyte character. On buffer overflow, drop the last multibyte
character to determine an accurate byte length.
---
src/xdisp.c | 15 ++++++++++++---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/src/xdisp.c b/src/xdisp.c
index 5b5cb3849fc..4933315166f 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -12587,10 +12587,19 @@ vmessage (const char *m, va_list ap)
ptrdiff_t len;
ptrdiff_t maxsize = FRAME_MESSAGE_BUF_SIZE (f);
USE_SAFE_ALLOCA;
- char *message_buf = SAFE_ALLOCA (maxsize + 1);
-
- len = doprnt (message_buf, maxsize, m, 0, ap);
+ char *message_buf = SAFE_ALLOCA (maxsize + MAX_MULTIBYTE_LENGTH);
+ len = doprnt (message_buf, maxsize + MAX_MULTIBYTE_LENGTH, m, 0, ap);
+ /* doprnt returns the buffer size minus one when it
+ truncated a multibyte sequence. Work around that by
+ truncating to the last valid multibyte head. */
+ if (len >= maxsize)
+ {
+ len = maxsize - 1;
+ while (!CHAR_HEAD_P (message_buf[len]))
+ len--;
+ message_buf[len] = 0;
+ }
message3 (make_string (message_buf, len));
SAFE_FREE ();
}
--
2.47.1
Pip
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.