* bug#9412: sprintf-related integer and memory overflow issues
@ 2011-08-30 22:42 Paul Eggert
2011-08-31 2:08 ` Chong Yidong
0 siblings, 1 reply; 8+ messages in thread
From: Paul Eggert @ 2011-08-30 22:42 UTC (permalink / raw)
To: 9412
Package: Emacs
Version: 24.0.50
Tags: patch
Here's a patch to the Emacs trunk to fix some sprintf-related integer
and memory overflow issues in Emacs proper. These bugs can cause the
wrong integer to be displayed, or a buffer overrun in sprintf output,
that sort of thing. Almost all the bugs can occur independently of
whether --with-wide-int is used. The bugs range from unlikely to
extremely unlikely in normal use (otherwise they would have been fixed
already....). The patch is (I hope) routine. I plan to install this
patch after some more internal testing.
Here's an example bug. Arrange for the DISPLAY environment variable
to use a host name that is longer than about 250 bytes. One way to do
this is to set DISPLAY='127.0.00000000000000.1:10.0' (but use 250 more zeros).
Run Emacs under X, and then arrange for the X server's connection to
be dropped. The Emacs function x_io_error_quitter will do this:
char buf[256];
sprintf (buf, "Connection lost to X server `%s'", DisplayString (display));
and the long host name will overrun BUF.
=== modified file 'src/ChangeLog'
--- src/ChangeLog 2011-08-30 18:15:53 +0000
+++ src/ChangeLog 2011-08-30 21:16:49 +0000
@@ -1,3 +1,109 @@
+2011-08-30 Paul Eggert <eggert@cs.ucla.edu>
+
+ sprintf-related integer and memory overflow issues.
+
+ * doprnt.c (doprnt): Support printing ptrdiff_t and intmax_t values.
+ (esprintf, esnprintf, exprintf, evxprintf): New functions.
+ * keyboard.c (command_loop_level): Now EMACS_INT, not int.
+ (cmd_error): kbd macro iterations count is now EMACS_INT, not int.
+ (modify_event_symbol): Do not assume that the length of
+ name_alist_or_stem is safe to alloca and fits in int.
+ (Fexecute_extended_command): Likewise for function name and binding.
+ (Frecursion_depth): Wrap around reliably on integer overflow.
+ * keymap.c (push_key_description): First arg is now EMACS_INT, not int,
+ since some callers pass EMACS_INT values.
+ (Fsingle_key_description): Don't crash if symbol name contains more
+ than MAX_ALLOCA bytes.
+ * minibuf.c (minibuf_level): Now EMACS_INT, not int.
+ (get_minibuffer): Arg is now EMACS_INT, not int.
+ * lisp.h (get_minibuffer, push_key_description): Reflect API changes.
+ (esprintf, esnprintf, exprintf, evxprintf): New decls.
+ * window.h (command_loop_level, minibuf_level): Reflect API changes.
+
+ * dbusbind.c (signature_cat): New function.
+ (xd_signature, Fdbus_register_signal):
+ Do not overrun buffer; instead, report string overflow.
+
+ * dispnew.c (add_window_display_history): Don't overrun buffer.
+ Truncate instead; this is OK since it's just a log.
+
+ * editfns.c (Fcurrent_time_zone): Don't overrun buffer
+ even if the time zone offset is outlandishly large.
+ Don't mishandle offset == INT_MIN.
+
+ * emacs.c (main) [NS_IMPL_COCOA]: Don't overrun buffer
+ when creating daemon; the previous buffer-overflow check was incorrect.
+
+ * eval.c (verror): Simplify by rewriting in terms of evxprintf,
+ which has the guts of the old verror function.
+
+ * filelock.c (lock_file_1, lock_file): Don't blindly alloca long name;
+ use SAFE_ALLOCA instead. Use esprintf to avoid int-overflow issues.
+
+ * font.c: Include <float.h>, for DBL_MAX_10_EXP.
+ (font_unparse_xlfd): Don't blindly alloca long strings.
+ Don't assume XINT result fits in int, or that XFLOAT_DATA * 10
+ fits in int, when using sprintf. Use single snprintf to count
+ length of string rather than counting it via multiple sprintfs;
+ that's simpler and more reliable.
+ (APPEND_SPRINTF): New macro.
+ (font_unparse_fcname): Use it to avoid sprintf buffer overrun.
+ (generate_otf_features) [0 && HAVE_LIBOTF]: Use esprintf, not
+ sprintf, in case result does not fit in int.
+
+ * fontset.c (num_auto_fontsets): Now printmax_t, not int.
+ (fontset_from_font): Print it.
+
+ * frame.c (tty_frame_count): Now printmax_t, not int.
+ (make_terminal_frame, set_term_frame_name): Print it.
+ (x_report_frame_params): In X, window IDs are unsigned long,
+ not signed long, so print them as unsigned.
+ (validate_x_resource_name): Check for implausibly long names,
+ and don't assume name length fits in 'int'.
+ (x_get_resource_string): Don't blindly alloca invocation name;
+ use SAFE_ALLOCA. Use esprintf, not sprintf, in case result does
+ not fit in int.
+
+ * gtkutil.c: Include <float.h>, for DBL_MAX_10_EXP.
+ (xg_check_special_colors, xg_set_geometry):
+ Make sprintf buffers a bit bigger, to avoid potential buffer overrun.
+
+ * lread.c (dir_warning): Don't blindly alloca buffer; use SAFE_ALLOCA.
+ Use esprintf, not sprintf, in case result does not fit in int.
+
+ * macros.c (executing_kbd_macro_iterations): Now EMACS_INT, not int.
+ (Fend_kbd_macro): Don't mishandle MOST_NEGATIVE_FIXNUM by treating
+ it as a large positive number.
+ (Fexecute_kbd_macro): Don't assume repeat count fits in int.
+ * macros.h (executing_kbd_macro_iterations): Now EMACS_INT, not int.
+
+ * nsterm.m ((NSSize)windowWillResize): Use esprintf, not sprintf,
+ in case result does not fit in int.
+
+ * print.c (float_to_string): Detect width overflow more reliably.
+ (print_object): Make sprintf buffer a bit bigger, to avoid potential
+ buffer overrun. Don't assume list length fits in 'int'. Treat
+ print length of 0 as 0, not as infinity; to be consistent with other
+ uses of print length in this function. Don't overflow print length
+ index. Don't assume hash table size fits in 'long', or that
+ vectorlike size fits in 'unsigned long'.
+
+ * process.c (make_process): Use printmax_t, not int, to format
+ process-name gensyms.
+
+ * term.c (produce_glyphless_glyph): Make sprintf buffer a bit bigger
+ to avoid potential buffer overrun.
+
+ * xfaces.c (x_update_menu_appearance): Don't overrun buffer
+ if X resource line is longer than 512 bytes.
+
+ * xfns.c (x_window): Make sprintf buffer a bit bigger
+ to avoid potential buffer overrun.
+
+ * xterm.c (x_io_error_quitter): Don't overrun sprintf buffer.
+
+ * xterm.h (x_check_errors): Add ATTRIBUTE_FORMAT_PRINTF.
+
2011-08-30 Eli Zaretskii <eliz@gnu.org>
* image.c (x_bitmap_pixmap): Cast to int to avoid compiler warnings.
=== modified file 'src/dbusbind.c'
--- src/dbusbind.c 2011-06-24 21:25:22 +0000
+++ src/dbusbind.c 2011-08-30 22:02:56 +0000
@@ -259,6 +259,18 @@
} \
while (0)
+/* Append to SIGNATURE a copy of X, making sure SIGNATURE does
+ not become too long. */
+static void
+signature_cat (char *signature, char const *x)
+{
+ ptrdiff_t siglen = strlen (signature);
+ ptrdiff_t xlen = strlen (x);
+ if (DBUS_MAXIMUM_SIGNATURE_LENGTH - xlen <= siglen)
+ string_overflow ();
+ strcat (signature, x);
+}
+
/* Compute SIGNATURE of OBJECT. It must have a form that it can be
used in dbus_message_iter_open_container. DTYPE is the DBusType
the object is related to. It is passed as argument, because it
@@ -271,6 +283,7 @@
{
unsigned int subtype;
Lisp_Object elt;
+ char const *subsig;
char x[DBUS_MAXIMUM_SIGNATURE_LENGTH];
elt = object;
@@ -328,12 +341,13 @@
if (NILP (elt))
{
subtype = DBUS_TYPE_STRING;
- strcpy (x, DBUS_TYPE_STRING_AS_STRING);
+ subsig = DBUS_TYPE_STRING_AS_STRING;
}
else
{
subtype = XD_OBJECT_TO_DBUS_TYPE (CAR_SAFE (elt));
xd_signature (x, subtype, dtype, CAR_SAFE (XD_NEXT_VALUE (elt)));
+ subsig = x;
}
/* If the element type is DBUS_TYPE_SIGNATURE, and this is the
@@ -342,7 +356,7 @@
if ((subtype == DBUS_TYPE_SIGNATURE)
&& STRINGP (CAR_SAFE (XD_NEXT_VALUE (elt)))
&& NILP (CDR_SAFE (XD_NEXT_VALUE (elt))))
- strcpy (x, SSDATA (CAR_SAFE (XD_NEXT_VALUE (elt))));
+ subsig = SSDATA (CAR_SAFE (XD_NEXT_VALUE (elt)));
while (!NILP (elt))
{
@@ -351,7 +365,10 @@
elt = CDR_SAFE (XD_NEXT_VALUE (elt));
}
- sprintf (signature, "%c%s", dtype, x);
+ if (esnprintf (signature, DBUS_MAXIMUM_SIGNATURE_LENGTH,
+ "%c%s", dtype, subsig)
+ == DBUS_MAXIMUM_SIGNATURE_LENGTH - 1)
+ string_overflow ();
break;
case DBUS_TYPE_VARIANT:
@@ -383,10 +400,10 @@
{
subtype = XD_OBJECT_TO_DBUS_TYPE (CAR_SAFE (elt));
xd_signature (x, subtype, dtype, CAR_SAFE (XD_NEXT_VALUE (elt)));
- strcat (signature, x);
+ signature_cat (signature, x);
elt = CDR_SAFE (XD_NEXT_VALUE (elt));
}
- strcat (signature, DBUS_STRUCT_END_CHAR_AS_STRING);
+ signature_cat (signature, DBUS_STRUCT_END_CHAR_AS_STRING);
break;
case DBUS_TYPE_DICT_ENTRY:
@@ -407,7 +424,7 @@
elt = XD_NEXT_VALUE (elt);
subtype = XD_OBJECT_TO_DBUS_TYPE (CAR_SAFE (elt));
xd_signature (x, subtype, dtype, CAR_SAFE (XD_NEXT_VALUE (elt)));
- strcat (signature, x);
+ signature_cat (signature, x);
if (!XD_BASIC_DBUS_TYPE (subtype))
wrong_type_argument (intern ("D-Bus"), CAR_SAFE (XD_NEXT_VALUE (elt)));
@@ -416,14 +433,14 @@
elt = CDR_SAFE (XD_NEXT_VALUE (elt));
subtype = XD_OBJECT_TO_DBUS_TYPE (CAR_SAFE (elt));
xd_signature (x, subtype, dtype, CAR_SAFE (XD_NEXT_VALUE (elt)));
- strcat (signature, x);
+ signature_cat (signature, x);
if (!NILP (CDR_SAFE (XD_NEXT_VALUE (elt))))
wrong_type_argument (intern ("D-Bus"),
CAR_SAFE (CDR_SAFE (XD_NEXT_VALUE (elt))));
/* Closing signature. */
- strcat (signature, DBUS_DICT_ENTRY_END_CHAR_AS_STRING);
+ signature_cat (signature, DBUS_DICT_ENTRY_END_CHAR_AS_STRING);
break;
default:
@@ -2026,7 +2043,7 @@
DBusConnection *connection;
ptrdiff_t i;
char rule[DBUS_MAXIMUM_MATCH_RULE_LENGTH];
- char x[DBUS_MAXIMUM_MATCH_RULE_LENGTH];
+ int rulelen;
DBusError derror;
/* Check parameters. */
@@ -2071,34 +2088,32 @@
connection = xd_initialize (bus, TRUE);
/* Create a rule to receive related signals. */
- sprintf (rule,
- "type='signal',interface='%s',member='%s'",
- SDATA (interface),
- SDATA (signal));
+ rulelen = esnprintf (rule, sizeof rule,
+ "type='signal',interface='%s',member='%s'",
+ SDATA (interface),
+ SDATA (signal));
/* Add unique name and path to the rule if they are non-nil. */
if (!NILP (uname))
- {
- sprintf (x, ",sender='%s'", SDATA (uname));
- strcat (rule, x);
- }
+ rulelen += esnprintf (rule + rulelen, sizeof rule - rulelen,
+ ",sender='%s'", SDATA (uname));
if (!NILP (path))
- {
- sprintf (x, ",path='%s'", SDATA (path));
- strcat (rule, x);
- }
+ rulelen += esnprintf (rule + rulelen, sizeof rule - rulelen,
+ ",path='%s'", SDATA (path));
/* Add arguments to the rule if they are non-nil. */
for (i = 6; i < nargs; ++i)
if (!NILP (args[i]))
{
CHECK_STRING (args[i]);
- sprintf (x, ",arg%"pD"d='%s'", i - 6,
- SDATA (args[i]));
- strcat (rule, x);
+ rulelen += esnprintf (rule + rulelen, sizeof rule - rulelen,
+ ",arg%"pD"d='%s'", i - 6, SDATA (args[i]));
}
+ if (rulelen == sizeof rule - 1)
+ string_overflow ();
+
/* Add the rule to the bus. */
dbus_error_init (&derror);
dbus_bus_add_match (connection, rule, &derror);
=== modified file 'src/dispnew.c'
--- src/dispnew.c 2011-08-24 21:20:36 +0000
+++ src/dispnew.c 2011-08-29 15:51:23 +0000
@@ -272,15 +272,16 @@
buf = redisplay_history[history_idx].trace;
++history_idx;
- sprintf (buf, "%"pMu": window %p (`%s')%s\n",
- history_tick++,
- w,
- ((BUFFERP (w->buffer)
- && STRINGP (BVAR (XBUFFER (w->buffer), name)))
- ? SSDATA (BVAR (XBUFFER (w->buffer), name))
- : "???"),
- paused_p ? " ***paused***" : "");
- strcat (buf, msg);
+ esnprintf (buf, sizeof redisplay_history[0].trace,
+ "%"pMu": window %p (`%s')%s\n%s",
+ history_tick++,
+ w,
+ ((BUFFERP (w->buffer)
+ && STRINGP (BVAR (XBUFFER (w->buffer), name)))
+ ? SSDATA (BVAR (XBUFFER (w->buffer), name))
+ : "???"),
+ paused_p ? " ***paused***" : "",
+ msg);
}
=== modified file 'src/doprnt.c'
--- src/doprnt.c 2011-07-07 02:14:52 +0000
+++ src/doprnt.c 2011-08-29 15:43:34 +0000
@@ -70,9 +70,9 @@
%<flags><width><precision><length>character
where flags is [+ -0], width is [0-9]+, precision is .[0-9]+, and length
- is empty or l or the value of the pI macro. Also, %% in a format
- stands for a single % in the output. A % that does not introduce a
- valid %-sequence causes undefined behavior.
+ is empty or l or the value of the pD or pI or pMd (sans "d") macros.
+ Also, %% in a format stands for a single % in the output. A % that
+ does not introduce a valid %-sequence causes undefined behavior.
The + flag character inserts a + before any positive number, while a space
inserts a space before any positive number; these flags only affect %d, %o,
@@ -85,8 +85,10 @@
modifier: it is supported for %d, %o, and %x conversions of integral
arguments, must immediately precede the conversion specifier, and means that
the respective argument is to be treated as `long int' or `unsigned long
- int'. Similarly, the value of the pI macro means to use EMACS_INT or
- EMACS_UINT and the empty length modifier means `int' or `unsigned int'.
+ int'. Similarly, the value of the pD macro means to use ptrdiff_t,
+ the value of the pI macro means to use EMACS_INT or EMACS_UINT, the
+ value of the pMd etc. macros means to use intmax_t or uintmax_t,
+ and the empty length modifier means `int' or `unsigned int'.
The width specifier supplies a lower limit for the length of the printed
representation. The padding, if any, normally goes on the left, but it goes
@@ -173,8 +175,17 @@
{
ptrdiff_t size_bound = 0;
EMACS_INT width; /* Columns occupied by STRING on display. */
- int long_flag = 0;
- int pIlen = sizeof pI - 1;
+ enum {
+ pDlen = sizeof pD - 1,
+ pIlen = sizeof pI - 1,
+ pMlen = sizeof pMd - 2
+ };
+ enum {
+ no_modifier, long_modifier, pD_modifier, pI_modifier, pM_modifier
+ } length_modifier = no_modifier;
+ static char const modifier_len[] = { 0, 1, pDlen, pIlen, pMlen };
+ int maxmlen = max (max (1, pDlen), max (pIlen, pMlen));
+ int mlen;
fmt++;
/* Copy this one %-spec into fmtcpy. */
@@ -213,19 +224,26 @@
fmt++;
}
- if (0 < pIlen && pIlen <= format_end - fmt
- && memcmp (fmt, pI, pIlen) == 0)
- {
- long_flag = 2;
- memcpy (string, fmt + 1, pIlen);
- string += pIlen;
- fmt += pIlen;
- }
- else if (fmt < format_end && *fmt == 'l')
- {
- long_flag = 1;
- *string++ = *++fmt;
- }
+ /* Check for the length modifiers in textual length order, so
+ that longer modifiers override shorter ones. */
+ for (mlen = 1; mlen <= maxmlen; mlen++)
+ {
+ if (format_end - fmt < mlen)
+ break;
+ if (mlen == 1 && *fmt == 'l')
+ length_modifier = long_modifier;
+ if (mlen == pDlen && memcmp (fmt, pD, pDlen) == 0)
+ length_modifier = pD_modifier;
+ if (mlen == pIlen && memcmp (fmt, pI, pIlen) == 0)
+ length_modifier = pI_modifier;
+ if (mlen == pMlen && memcmp (fmt, pMd, pMlen) == 0)
+ length_modifier = pM_modifier;
+ }
+
+ mlen = modifier_len[length_modifier];
+ memcpy (string, fmt + 1, mlen);
+ string += mlen;
+ fmt += mlen;
*string = 0;
/* Make the size bound large enough to handle floating point formats
@@ -252,55 +270,78 @@
/* case 'b': */
case 'l':
case 'd':
- {
- int i;
- long l;
-
- if (1 < long_flag)
- {
- EMACS_INT ll = va_arg (ap, EMACS_INT);
- sprintf (sprintf_buffer, fmtcpy, ll);
- }
- else if (long_flag)
- {
- l = va_arg(ap, long);
- sprintf (sprintf_buffer, fmtcpy, l);
- }
- else
- {
- i = va_arg(ap, int);
- sprintf (sprintf_buffer, fmtcpy, i);
- }
- /* Now copy into final output, truncating as necessary. */
- string = sprintf_buffer;
- goto doit;
- }
+ switch (length_modifier)
+ {
+ case no_modifier:
+ {
+ int v = va_arg (ap, int);
+ sprintf (sprintf_buffer, fmtcpy, v);
+ }
+ break;
+ case long_modifier:
+ {
+ long v = va_arg (ap, long);
+ sprintf (sprintf_buffer, fmtcpy, v);
+ }
+ break;
+ case pD_modifier:
+ signed_pD_modifier:
+ {
+ ptrdiff_t v = va_arg (ap, ptrdiff_t);
+ sprintf (sprintf_buffer, fmtcpy, v);
+ }
+ break;
+ case pI_modifier:
+ {
+ EMACS_INT v = va_arg (ap, EMACS_INT);
+ sprintf (sprintf_buffer, fmtcpy, v);
+ }
+ break;
+ case pM_modifier:
+ {
+ intmax_t v = va_arg (ap, intmax_t);
+ sprintf (sprintf_buffer, fmtcpy, v);
+ }
+ break;
+ }
+ /* Now copy into final output, truncating as necessary. */
+ string = sprintf_buffer;
+ goto doit;
case 'o':
case 'x':
- {
- unsigned u;
- unsigned long ul;
-
- if (1 < long_flag)
- {
- EMACS_UINT ull = va_arg (ap, EMACS_UINT);
- sprintf (sprintf_buffer, fmtcpy, ull);
- }
- else if (long_flag)
- {
- ul = va_arg(ap, unsigned long);
- sprintf (sprintf_buffer, fmtcpy, ul);
- }
- else
- {
- u = va_arg(ap, unsigned);
- sprintf (sprintf_buffer, fmtcpy, u);
- }
- /* Now copy into final output, truncating as necessary. */
- string = sprintf_buffer;
- goto doit;
- }
+ switch (length_modifier)
+ {
+ case no_modifier:
+ {
+ unsigned v = va_arg (ap, unsigned);
+ sprintf (sprintf_buffer, fmtcpy, v);
+ }
+ break;
+ case long_modifier:
+ {
+ unsigned long v = va_arg (ap, unsigned long);
+ sprintf (sprintf_buffer, fmtcpy, v);
+ }
+ break;
+ case pD_modifier:
+ goto signed_pD_modifier;
+ case pI_modifier:
+ {
+ EMACS_UINT v = va_arg (ap, EMACS_UINT);
+ sprintf (sprintf_buffer, fmtcpy, v);
+ }
+ break;
+ case pM_modifier:
+ {
+ uintmax_t v = va_arg (ap, uintmax_t);
+ sprintf (sprintf_buffer, fmtcpy, v);
+ }
+ break;
+ }
+ /* Now copy into final output, truncating as necessary. */
+ string = sprintf_buffer;
+ goto doit;
case 'f':
case 'e':
@@ -426,3 +467,82 @@
SAFE_FREE ();
return bufptr - buffer;
}
+
+/* Format to an unbounded buffer BUF. This is like sprintf, except it
+ is not limited to returning an 'int' so it doesn't have a silly 2
+ GiB limit on typical 64-bit hosts. However, it is limited to the
+ Emacs-style formats that doprnt supports.
+
+ Return the number of bytes put into BUF, excluding the terminating
+ '\0'. */
+ptrdiff_t
+esprintf (char *buf, char const *format, ...)
+{
+ ptrdiff_t nbytes;
+ va_list ap;
+ va_start (ap, format);
+ nbytes = doprnt (buf, TYPE_MAXIMUM (ptrdiff_t), format, 0, ap);
+ va_end (ap);
+ return nbytes;
+}
+
+/* Format to a buffer BUF of positive size BUFSIZE. This is like
+ snprintf, except it is not limited to returning an 'int' so it
+ doesn't have a silly 2 GiB limit on typical 64-bit hosts. However,
+ it is limited to the Emacs-style formats that doprnt supports, and
+ BUFSIZE must be positive.
+
+ Return the number of bytes put into BUF, excluding the terminating
+ '\0'. Unlike snprintf, always return a nonnegative value less than
+ BUFSIZE; if the output is truncated, return BUFSIZE - 1, which is
+ the length of the truncated output. */
+ptrdiff_t
+esnprintf (char *buf, ptrdiff_t bufsize, char const *format, ...)
+{
+ ptrdiff_t nbytes;
+ va_list ap;
+ va_start (ap, format);
+ nbytes = doprnt (buf, bufsize, format, 0, ap);
+ va_end (ap);
+ return nbytes;
+}
+
+/* Format to buffer *BUF of positive size *BUFSIZE, reallocating *BUF
+ and updating *BUFSIZE if the buffer is too small, and otherwise
+ behaving line esprintf. When reallocating, free *BUF unless it is
+ equal to NONHEAPBUF, and if BUFSIZE_MAX is nonnegative then signal
+ memory exhaustion instead of growing the buffer size past
+ BUFSIZE_MAX. */
+ptrdiff_t
+exprintf (char **buf, ptrdiff_t *bufsize,
+ char const *nonheapbuf, ptrdiff_t bufsize_max,
+ char const *format, ...)
+{
+ ptrdiff_t nbytes;
+ va_list ap;
+ va_start (ap, format);
+ nbytes = evxprintf (buf, bufsize, nonheapbuf, bufsize_max, format, ap);
+ va_end (ap);
+ return nbytes;
+}
+
+/* Act like exprintf, except take a va_list. */
+ptrdiff_t
+evxprintf (char **buf, ptrdiff_t *bufsize,
+ char const *nonheapbuf, ptrdiff_t bufsize_max,
+ char const *format, va_list ap)
+{
+ for (;;)
+ {
+ ptrdiff_t nbytes;
+ va_list ap_copy;
+ va_copy (ap_copy, ap);
+ nbytes = doprnt (*buf, *bufsize, format, 0, ap_copy);
+ va_end (ap_copy);
+ if (nbytes < *bufsize - 1)
+ return nbytes;
+ if (*buf != nonheapbuf)
+ xfree (*buf);
+ *buf = xpalloc (NULL, bufsize, 1, bufsize_max, 1);
+ }
+}
=== modified file 'src/editfns.c'
--- src/editfns.c 2011-08-19 06:11:38 +0000
+++ src/editfns.c 2011-08-29 15:53:21 +0000
@@ -2014,7 +2014,7 @@
{
int offset = tm_diff (t, &gmt);
char *s = 0;
- char buf[6];
+ char buf[sizeof "+00" + INT_STRLEN_BOUND (int)];
#ifdef HAVE_TM_ZONE
if (t->tm_zone)
@@ -2029,7 +2029,8 @@
if (!s)
{
/* No local time zone name is available; use "+-NNNN" instead. */
- int am = (offset < 0 ? -offset : offset) / 60;
+ int m = offset / 60;
+ int am = offset < 0 ? - m : m;
sprintf (buf, "%c%02d%02d", (offset < 0 ? '-' : '+'), am/60, am%60);
s = buf;
}
=== modified file 'src/emacs.c'
--- src/emacs.c 2011-08-05 02:19:34 +0000
+++ src/emacs.c 2011-08-29 15:56:20 +0000
@@ -1068,15 +1068,17 @@
if (!dname_arg || !strchr (dname_arg, '\n'))
{ /* In orig, child: now exec w/special daemon name. */
char fdStr[80];
+ int fdStrlen =
+ snprintf (fdStr, sizeof fdStr,
+ "--daemon=\n%d,%d\n%s", daemon_pipe[0],
+ daemon_pipe[1], dname_arg ? dname_arg : "");
- if (dname_arg && strlen (dname_arg) > 70)
+ if (! (0 <= fdStrlen && fdStrlen < sizeof fdStr))
{
fprintf (stderr, "daemon: child name too long\n");
exit (1);
}
- sprintf (fdStr, "--daemon=\n%d,%d\n%s", daemon_pipe[0],
- daemon_pipe[1], dname_arg ? dname_arg : "");
argv[skip_args] = fdStr;
execv (argv[0], argv);
=== modified file 'src/eval.c'
--- src/eval.c 2011-08-24 21:20:36 +0000
+++ src/eval.c 2011-08-29 16:01:33 +0000
@@ -1951,35 +1951,11 @@
char buf[4000];
ptrdiff_t size = sizeof buf;
ptrdiff_t size_max = STRING_BYTES_BOUND + 1;
- char const *m_end = m + strlen (m);
char *buffer = buf;
ptrdiff_t used;
Lisp_Object string;
- while (1)
- {
- va_list ap_copy;
- va_copy (ap_copy, ap);
- used = doprnt (buffer, size, m, m_end, ap_copy);
- va_end (ap_copy);
-
- /* Note: the -1 below is because `doprnt' returns the number of bytes
- excluding the terminating null byte, and it always terminates with a
- null byte, even when producing a truncated message. */
- if (used < size - 1)
- break;
- if (size <= size_max / 2)
- size *= 2;
- else if (size < size_max)
- size = size_max;
- else
- break; /* and leave the message truncated */
-
- if (buffer != buf)
- xfree (buffer);
- buffer = (char *) xmalloc (size);
- }
-
+ used = evxprintf (&buffer, &size, buf, size_max, m, ap);
string = make_string (buffer, used);
if (buffer != buf)
xfree (buffer);
=== modified file 'src/filelock.c'
--- src/filelock.c 2011-07-07 21:52:44 +0000
+++ src/filelock.c 2011-08-29 16:48:19 +0000
@@ -341,6 +341,9 @@
const char *user_name;
const char *host_name;
char *lock_info_str;
+ ptrdiff_t lock_info_size;
+ int symlink_errno;
+ USE_SAFE_ALLOCA;
/* Call this first because it can GC. */
boot = get_boot_time ();
@@ -353,17 +356,14 @@
host_name = SSDATA (Fsystem_name ());
else
host_name = "";
- lock_info_str = (char *)alloca (strlen (user_name) + strlen (host_name)
- + 2 * INT_STRLEN_BOUND (printmax_t)
- + sizeof "@.:");
+ lock_info_size = (strlen (user_name) + strlen (host_name)
+ + 2 * INT_STRLEN_BOUND (printmax_t)
+ + sizeof "@.:");
+ SAFE_ALLOCA (lock_info_str, char *, lock_info_size);
pid = getpid ();
- if (boot)
- sprintf (lock_info_str, "%s@%s.%"pMd":%"pMd,
- user_name, host_name, pid, boot);
- else
- sprintf (lock_info_str, "%s@%s.%"pMd,
- user_name, host_name, pid);
+ esprintf (lock_info_str, boot ? "%s@%s.%"pMd":%"pMd : "%s@%s.%"pMd,
+ user_name, host_name, pid, boot);
err = symlink (lock_info_str, lfname);
if (errno == EEXIST && force)
@@ -372,6 +372,9 @@
err = symlink (lock_info_str, lfname);
}
+ symlink_errno = errno;
+ SAFE_FREE ();
+ errno = symlink_errno;
return err == 0;
}
@@ -541,9 +544,11 @@
{
register Lisp_Object attack, orig_fn, encoded_fn;
register char *lfname, *locker;
+ ptrdiff_t locker_size;
lock_info_type lock_info;
printmax_t pid;
struct gcpro gcpro1;
+ USE_SAFE_ALLOCA;
/* Don't do locking while dumping Emacs.
Uncompressing wtmp files uses call-process, which does not work
@@ -580,15 +585,17 @@
return;
/* Else consider breaking the lock */
- locker = (char *) alloca (strlen (lock_info.user) + strlen (lock_info.host)
- + INT_STRLEN_BOUND (printmax_t)
- + sizeof "@ (pid )");
+ locker_size = (strlen (lock_info.user) + strlen (lock_info.host)
+ + INT_STRLEN_BOUND (printmax_t)
+ + sizeof "@ (pid )");
+ SAFE_ALLOCA (locker, char *, locker_size);
pid = lock_info.pid;
- sprintf (locker, "%s@%s (pid %"pMd")",
- lock_info.user, lock_info.host, pid);
+ esprintf (locker, "%s@%s (pid %"pMd")",
+ lock_info.user, lock_info.host, pid);
FREE_LOCK_INFO (lock_info);
attack = call2 (intern ("ask-user-about-lock"), fn, build_string (locker));
+ SAFE_FREE ();
if (!NILP (attack))
/* User says take the lock */
{
=== modified file 'src/font.c'
--- src/font.c 2011-07-10 08:20:10 +0000
+++ src/font.c 2011-08-29 20:57:42 +0000
@@ -21,6 +21,7 @@
along with GNU Emacs. If not, see <http://www.gnu.org/licenses/>. */
#include <config.h>
+#include <float.h>
#include <stdio.h>
#include <ctype.h>
#include <setjmp.h>
@@ -1180,7 +1181,7 @@
char *p;
const char *f[XLFD_REGISTRY_INDEX + 1];
Lisp_Object val;
- int i, j, len = 0;
+ int i, j, len;
font_assert (FONTP (font));
@@ -1195,9 +1196,9 @@
if (NILP (val))
{
if (j == XLFD_REGISTRY_INDEX)
- f[j] = "*-*", len += 4;
+ f[j] = "*-*";
else
- f[j] = "*", len += 2;
+ f[j] = "*";
}
else
{
@@ -1207,21 +1208,15 @@
&& ! strchr (SSDATA (val), '-'))
{
/* Change "jisx0208*" and "jisx0208" to "jisx0208*-*". */
- if (SDATA (val)[SBYTES (val) - 1] == '*')
- {
- f[j] = p = alloca (SBYTES (val) + 3);
- sprintf (p, "%s-*", SDATA (val));
- len += SBYTES (val) + 3;
- }
- else
- {
- f[j] = p = alloca (SBYTES (val) + 4);
- sprintf (p, "%s*-*", SDATA (val));
- len += SBYTES (val) + 4;
- }
+ ptrdiff_t alloc = SBYTES (val) + 4;
+ if (nbytes <= alloc)
+ return -1;
+ f[j] = p = alloca (alloc);
+ sprintf (p, "%s%s-*", SDATA (val),
+ "*" + (SDATA (val)[SBYTES (val) - 1] == '*'));
}
else
- f[j] = SSDATA (val), len += SBYTES (val) + 1;
+ f[j] = SSDATA (val);
}
}
@@ -1230,11 +1225,11 @@
{
val = font_style_symbolic (font, i, 0);
if (NILP (val))
- f[j] = "*", len += 2;
+ f[j] = "*";
else
{
val = SYMBOL_NAME (val);
- f[j] = SSDATA (val), len += SBYTES (val) + 1;
+ f[j] = SSDATA (val);
}
}
@@ -1242,64 +1237,62 @@
font_assert (NUMBERP (val) || NILP (val));
if (INTEGERP (val))
{
- i = XINT (val);
- if (i <= 0)
- i = pixel_size;
- if (i > 0)
+ EMACS_INT v = XINT (val);
+ if (v <= 0)
+ v = pixel_size;
+ if (v > 0)
{
- f[XLFD_PIXEL_INDEX] = p = alloca (22);
- len += sprintf (p, "%d-*", i) + 1;
+ f[XLFD_PIXEL_INDEX] = p =
+ alloca (sizeof "-*" + INT_STRLEN_BOUND (EMACS_INT));
+ sprintf (p, "%"pI"d-*", v);
}
else
- f[XLFD_PIXEL_INDEX] = "*-*", len += 4;
+ f[XLFD_PIXEL_INDEX] = "*-*";
}
else if (FLOATP (val))
{
- i = XFLOAT_DATA (val) * 10;
- f[XLFD_PIXEL_INDEX] = p = alloca (12);
- len += sprintf (p, "*-%d", i) + 1;
+ double v = XFLOAT_DATA (val) * 10;
+ f[XLFD_PIXEL_INDEX] = p = alloca (sizeof "*-" + 1 + DBL_MAX_10_EXP + 1);
+ sprintf (p, "*-%.0f", v);
}
else
- f[XLFD_PIXEL_INDEX] = "*-*", len += 4;
+ f[XLFD_PIXEL_INDEX] = "*-*";
if (INTEGERP (AREF (font, FONT_DPI_INDEX)))
{
- i = XINT (AREF (font, FONT_DPI_INDEX));
- f[XLFD_RESX_INDEX] = p = alloca (22);
- len += sprintf (p, "%d-%d", i, i) + 1;
+ EMACS_INT v = XINT (AREF (font, FONT_DPI_INDEX));
+ f[XLFD_RESX_INDEX] = p =
+ alloca (sizeof "-" + 2 * INT_STRLEN_BOUND (EMACS_INT));
+ sprintf (p, "%"pI"d-%"pI"d", v, v);
}
else
- f[XLFD_RESX_INDEX] = "*-*", len += 4;
+ f[XLFD_RESX_INDEX] = "*-*";
if (INTEGERP (AREF (font, FONT_SPACING_INDEX)))
{
- int spacing = XINT (AREF (font, FONT_SPACING_INDEX));
+ EMACS_INT spacing = XINT (AREF (font, FONT_SPACING_INDEX));
f[XLFD_SPACING_INDEX] = (spacing <= FONT_SPACING_PROPORTIONAL ? "p"
: spacing <= FONT_SPACING_DUAL ? "d"
: spacing <= FONT_SPACING_MONO ? "m"
: "c");
- len += 2;
}
else
- f[XLFD_SPACING_INDEX] = "*", len += 2;
+ f[XLFD_SPACING_INDEX] = "*";
if (INTEGERP (AREF (font, FONT_AVGWIDTH_INDEX)))
{
- f[XLFD_AVGWIDTH_INDEX] = p = alloca (22);
- len += sprintf (p, "%"pI"d",
- XINT (AREF (font, FONT_AVGWIDTH_INDEX))) + 1;
+ f[XLFD_AVGWIDTH_INDEX] = p = alloca (INT_BUFSIZE_BOUND (EMACS_INT));
+ sprintf (p, "%"pI"d", XINT (AREF (font, FONT_AVGWIDTH_INDEX)));
}
else
- f[XLFD_AVGWIDTH_INDEX] = "*", len += 2;
- len++; /* for terminating '\0'. */
- if (len >= nbytes)
- return -1;
- return sprintf (name, "-%s-%s-%s-%s-%s-%s-%s-%s-%s-%s-%s",
+ f[XLFD_AVGWIDTH_INDEX] = "*";
+ len = snprintf (name, nbytes, "-%s-%s-%s-%s-%s-%s-%s-%s-%s-%s-%s",
f[XLFD_FOUNDRY_INDEX], f[XLFD_FAMILY_INDEX],
f[XLFD_WEIGHT_INDEX], f[XLFD_SLANT_INDEX],
f[XLFD_SWIDTH_INDEX], f[XLFD_ADSTYLE_INDEX],
f[XLFD_PIXEL_INDEX], f[XLFD_RESX_INDEX],
f[XLFD_SPACING_INDEX], f[XLFD_AVGWIDTH_INDEX],
f[XLFD_REGISTRY_INDEX]);
+ return len < nbytes ? len : -1;
}
/* Parse NAME (null terminated) and store information in FONT
@@ -1553,23 +1546,19 @@
font_unparse_fcname (Lisp_Object font, int pixel_size, char *name, int nbytes)
{
Lisp_Object family, foundry;
- Lisp_Object tail, val;
+ Lisp_Object val;
int point_size;
int i;
- ptrdiff_t len = 1;
char *p;
+ char *lim;
Lisp_Object styles[3];
const char *style_names[3] = { "weight", "slant", "width" };
- char work[256];
family = AREF (font, FONT_FAMILY_INDEX);
if (! NILP (family))
{
if (SYMBOLP (family))
- {
- family = SYMBOL_NAME (family);
- len += SBYTES (family);
- }
+ family = SYMBOL_NAME (family);
else
family = Qnil;
}
@@ -1580,7 +1569,6 @@
if (XINT (val) != 0)
pixel_size = XINT (val);
point_size = -1;
- len += 21; /* for ":pixelsize=NUM" */
}
else
{
@@ -1588,80 +1576,54 @@
abort ();
pixel_size = -1;
point_size = (int) XFLOAT_DATA (val);
- len += 11; /* for "-NUM" */
}
foundry = AREF (font, FONT_FOUNDRY_INDEX);
if (! NILP (foundry))
{
if (SYMBOLP (foundry))
- {
- foundry = SYMBOL_NAME (foundry);
- len += 9 + SBYTES (foundry); /* ":foundry=NAME" */
- }
+ foundry = SYMBOL_NAME (foundry);
else
foundry = Qnil;
}
for (i = 0; i < 3; i++)
- {
- styles[i] = font_style_symbolic (font, FONT_WEIGHT_INDEX + i, 0);
- if (! NILP (styles[i]))
- len += sprintf (work, ":%s=%s", style_names[i],
- SDATA (SYMBOL_NAME (styles[i])));
- }
-
- if (INTEGERP (AREF (font, FONT_DPI_INDEX)))
- len += sprintf (work, ":dpi=%"pI"d", XINT (AREF (font, FONT_DPI_INDEX)));
- if (INTEGERP (AREF (font, FONT_SPACING_INDEX)))
- len += strlen (":spacing=100");
- if (INTEGERP (AREF (font, FONT_AVGWIDTH_INDEX)))
- len += strlen (":scalable=false"); /* or ":scalable=true" */
- for (tail = AREF (font, FONT_EXTRA_INDEX); CONSP (tail); tail = XCDR (tail))
- {
- Lisp_Object key = XCAR (XCAR (tail)), value = XCDR (XCAR (tail));
-
- len += SBYTES (SYMBOL_NAME (key)) + 1; /* for :KEY= */
- if (STRINGP (value))
- len += SBYTES (value);
- else if (INTEGERP (value))
- len += sprintf (work, "%"pI"d", XINT (value));
- else if (SYMBOLP (value))
- len += (NILP (value) ? 5 : 4); /* for "false" or "true" */
- }
-
- if (len > nbytes)
- return -1;
+ styles[i] = font_style_symbolic (font, FONT_WEIGHT_INDEX + i, 0);
+
p = name;
+ lim = name + nbytes;
+# define APPEND_SNPRINTF(args) \
+ do { \
+ int len = snprintf args; \
+ if (! (0 <= len && len < lim - p)) \
+ return -1; \
+ p += len; \
+ } while (0)
if (! NILP (family))
- p += sprintf (p, "%s", SDATA (family));
+ APPEND_SNPRINTF ((p, lim - p, "%s", SSDATA (family)));
if (point_size > 0)
- {
- if (p == name)
- p += sprintf (p, "%d", point_size);
- else
- p += sprintf (p, "-%d", point_size);
- }
+ APPEND_SNPRINTF ((p, lim - p, "-%d" + (p == name), point_size));
else if (pixel_size > 0)
- p += sprintf (p, ":pixelsize=%d", pixel_size);
+ APPEND_SNPRINTF ((p, lim - p, ":pixelsize=%d", pixel_size));
if (! NILP (AREF (font, FONT_FOUNDRY_INDEX)))
- p += sprintf (p, ":foundry=%s",
- SDATA (SYMBOL_NAME (AREF (font, FONT_FOUNDRY_INDEX))));
+ APPEND_SNPRINTF ((p, lim - p, ":foundry=%s",
+ SSDATA (SYMBOL_NAME (AREF (font,
+ FONT_FOUNDRY_INDEX)))));
for (i = 0; i < 3; i++)
if (! NILP (styles[i]))
- p += sprintf (p, ":%s=%s", style_names[i],
- SDATA (SYMBOL_NAME (styles[i])));
+ APPEND_SNPRINTF ((p, lim - p, ":%s=%s", style_names[i],
+ SSDATA (SYMBOL_NAME (styles[i]))));
if (INTEGERP (AREF (font, FONT_DPI_INDEX)))
- p += sprintf (p, ":dpi=%"pI"d", XINT (AREF (font, FONT_DPI_INDEX)));
+ APPEND_SNPRINTF ((p, lim - p, ":dpi=%"pI"d",
+ XINT (AREF (font, FONT_DPI_INDEX))));
if (INTEGERP (AREF (font, FONT_SPACING_INDEX)))
- p += sprintf (p, ":spacing=%"pI"d", XINT (AREF (font, FONT_SPACING_INDEX)));
+ APPEND_SNPRINTF ((p, lim - p, ":spacing=%"pI"d",
+ XINT (AREF (font, FONT_SPACING_INDEX))));
if (INTEGERP (AREF (font, FONT_AVGWIDTH_INDEX)))
- {
- if (XINT (AREF (font, FONT_AVGWIDTH_INDEX)) == 0)
- p += sprintf (p, ":scalable=true");
- else
- p += sprintf (p, ":scalable=false");
- }
+ APPEND_SNPRINTF ((p, lim - p,
+ (XINT (AREF (font, FONT_AVGWIDTH_INDEX)) == 0
+ ? ":scalable=true"
+ : ":scalable=false")));
return (p - name);
}
@@ -1952,12 +1914,12 @@
else if (! asterisk)
{
val = SYMBOL_NAME (val);
- p += sprintf (p, "%s", SDATA (val));
+ p += esprintf (p, "%s", SDATA (val));
}
else
{
val = SYMBOL_NAME (val);
- p += sprintf (p, "~%s", SDATA (val));
+ p += esprintf (p, "~%s", SDATA (val));
}
}
if (CONSP (spec))
=== modified file 'src/fontset.c'
--- src/fontset.c 2011-08-09 22:13:11 +0000
+++ src/fontset.c 2011-08-29 18:48:24 +0000
@@ -1700,7 +1700,7 @@
static Lisp_Object auto_fontset_alist;
/* Number of automatically created fontsets. */
-static int num_auto_fontsets;
+static printmax_t num_auto_fontsets;
/* Retun a fontset synthesized from FONT-OBJECT. This is called from
x_new_font when FONT-OBJECT is used for the default ASCII font of a
@@ -1727,9 +1727,9 @@
alias = intern ("fontset-startup");
else
{
- char temp[32];
+ char temp[sizeof "fontset-auto" + INT_STRLEN_BOUND (printmax_t)];
- sprintf (temp, "fontset-auto%d", num_auto_fontsets - 1);
+ sprintf (temp, "fontset-auto%"pMd, num_auto_fontsets - 1);
alias = intern (temp);
}
fontset_spec = copy_font_spec (font_spec);
=== modified file 'src/frame.c'
--- src/frame.c 2011-08-14 06:40:45 +0000
+++ src/frame.c 2011-08-29 18:52:26 +0000
@@ -497,7 +497,7 @@
\f
/* Construct a frame that refers to a terminal. */
-static int tty_frame_count;
+static printmax_t tty_frame_count;
struct frame *
make_initial_frame (void)
@@ -551,7 +551,7 @@
{
register struct frame *f;
Lisp_Object frame;
- char name[20];
+ char name[sizeof "F" + INT_STRLEN_BOUND (printmax_t)];
if (!terminal->name)
error ("Terminal is not live, can't create new frames on it");
@@ -562,7 +562,7 @@
Vframe_list = Fcons (frame, Vframe_list);
tty_frame_count++;
- sprintf (name, "F%d", tty_frame_count);
+ sprintf (name, "F%"pMd, tty_frame_count);
f->name = build_string (name);
f->visible = 1; /* FRAME_SET_VISIBLE wd set frame_garbaged. */
@@ -2074,7 +2074,7 @@
/* If NAME is nil, set the name to F<num>. */
if (NILP (name))
{
- char namebuf[20];
+ char namebuf[sizeof "F" + INT_STRLEN_BOUND (printmax_t)];
/* Check for no change needed in this very common case
before we do any consing. */
@@ -2083,7 +2083,7 @@
return;
tty_frame_count++;
- sprintf (namebuf, "F%d", tty_frame_count);
+ sprintf (namebuf, "F%"pMd, tty_frame_count);
name = build_string (namebuf);
}
else
@@ -3065,6 +3065,7 @@
{
char buf[16];
Lisp_Object tem;
+ unsigned long w;
/* Represent negative positions (off the top or left screen edge)
in a way that Fmodify_frame_parameters will understand correctly. */
@@ -3097,7 +3098,8 @@
for non-toolkit scroll bar.
ruler-mode.el depends on this. */
: Qnil));
- sprintf (buf, "%ld", (long) FRAME_X_WINDOW (f));
+ w = FRAME_X_WINDOW (f);
+ sprintf (buf, "%lu", w);
store_in_alist (alistptr, Qwindow_id,
build_string (buf));
#ifdef HAVE_X_WINDOWS
@@ -3105,7 +3107,10 @@
/* Tooltip frame may not have this widget. */
if (FRAME_X_OUTPUT (f)->widget)
#endif
- sprintf (buf, "%ld", (long) FRAME_OUTER_WINDOW (f));
+ {
+ w = FRAME_OUTER_WINDOW (f);
+ sprintf (buf, "%lu", w);
+ }
store_in_alist (alistptr, Qouter_window_id,
build_string (buf));
#endif
@@ -3576,13 +3581,13 @@
void
validate_x_resource_name (void)
{
- int len = 0;
+ ptrdiff_t len = 0;
/* Number of valid characters in the resource name. */
- int good_count = 0;
+ ptrdiff_t good_count = 0;
/* Number of invalid characters in the resource name. */
- int bad_count = 0;
+ ptrdiff_t bad_count = 0;
Lisp_Object new;
- int i;
+ ptrdiff_t i;
if (!STRINGP (Vx_resource_class))
Vx_resource_class = build_string (EMACS_CLASS);
@@ -3615,8 +3620,9 @@
if (bad_count == 0)
return;
- /* If name is entirely invalid, or nearly so, use `emacs'. */
- if (good_count < 2)
+ /* If name is entirely invalid, or nearly so, or is so implausibly
+ large that alloca might not work, use `emacs'. */
+ if (good_count < 2 || MAX_ALLOCA - sizeof ".customization" < len)
{
Vx_resource_name = build_string ("emacs");
return;
@@ -3745,20 +3751,24 @@
{
char *name_key;
char *class_key;
+ char *result;
struct frame *sf = SELECTED_FRAME ();
+ ptrdiff_t invocation_namelen = SBYTES (Vinvocation_name);
+ USE_SAFE_ALLOCA;
/* Allocate space for the components, the dots which separate them,
and the final '\0'. */
- name_key = (char *) alloca (SBYTES (Vinvocation_name)
- + strlen (attribute) + 2);
+ SAFE_ALLOCA (name_key, char *, invocation_namelen + strlen (attribute) + 2);
class_key = (char *) alloca ((sizeof (EMACS_CLASS) - 1)
+ strlen (class) + 2);
- sprintf (name_key, "%s.%s", SSDATA (Vinvocation_name), attribute);
+ esprintf (name_key, "%s.%s", SSDATA (Vinvocation_name), attribute);
sprintf (class_key, "%s.%s", EMACS_CLASS, class);
- return x_get_string_resource (FRAME_X_DISPLAY_INFO (sf)->xrdb,
- name_key, class_key);
+ result = x_get_string_resource (FRAME_X_DISPLAY_INFO (sf)->xrdb,
+ name_key, class_key);
+ SAFE_FREE ();
+ return result;
}
#endif
=== modified file 'src/gtkutil.c'
--- src/gtkutil.c 2011-08-05 02:19:34 +0000
+++ src/gtkutil.c 2011-08-29 20:57:42 +0000
@@ -20,6 +20,7 @@
#include <config.h>
#ifdef USE_GTK
+#include <float.h>
#include <signal.h>
#include <stdio.h>
#include <setjmp.h>
@@ -567,7 +568,7 @@
GtkStyleContext *gsty
= gtk_widget_get_style_context (FRAME_GTK_OUTER_WIDGET (f));
GdkRGBA col;
- char buf[64];
+ char buf[sizeof "rgbi://" + 3 * (DBL_MAX_10_EXP + sizeof "-1.000000" - 1)];
int state = GTK_STATE_FLAG_SELECTED|GTK_STATE_FLAG_FOCUSED;
if (get_fg)
gtk_style_context_get_color (gsty, state, &col);
@@ -797,7 +798,7 @@
int xneg = f->size_hint_flags & XNegative;
int top = f->top_pos;
int yneg = f->size_hint_flags & YNegative;
- char geom_str[32];
+ char geom_str[sizeof "=x--" + 4 * INT_STRLEN_BOUND (int)];
if (xneg)
left = -left;
=== modified file 'src/keyboard.c'
--- src/keyboard.c 2011-08-05 02:19:34 +0000
+++ src/keyboard.c 2011-08-29 15:43:34 +0000
@@ -196,7 +196,7 @@
int quit_char;
/* Current depth in recursive edits. */
-int command_loop_level;
+EMACS_INT command_loop_level;
/* If not Qnil, this is a switch-frame event which we decided to put
off until the end of a key sequence. This should be read as the
@@ -998,7 +998,8 @@
cmd_error (Lisp_Object data)
{
Lisp_Object old_level, old_length;
- char macroerror[50];
+ char macroerror[sizeof "After..kbd macro iterations: "
+ + INT_STRLEN_BOUND (EMACS_INT)];
#ifdef HAVE_WINDOW_SYSTEM
if (display_hourglass_p)
@@ -1010,7 +1011,7 @@
if (executing_kbd_macro_iterations == 1)
sprintf (macroerror, "After 1 kbd macro iteration: ");
else
- sprintf (macroerror, "After %d kbd macro iterations: ",
+ sprintf (macroerror, "After %"pI"d kbd macro iterations: ",
executing_kbd_macro_iterations);
}
else
@@ -6463,11 +6464,15 @@
value = Fcdr_safe (Fassq (symbol_int, name_alist_or_stem));
else if (STRINGP (name_alist_or_stem))
{
- int len = SBYTES (name_alist_or_stem);
- char *buf = (char *) alloca (len + 50);
- sprintf (buf, "%s-%"pI"d", SDATA (name_alist_or_stem),
- XINT (symbol_int) + 1);
+ char *buf;
+ ptrdiff_t len = (SBYTES (name_alist_or_stem)
+ + sizeof "-" + INT_STRLEN_BOUND (EMACS_INT));
+ USE_SAFE_ALLOCA;
+ SAFE_ALLOCA (buf, char *, len);
+ esprintf (buf, "%s-%"pI"d", SDATA (name_alist_or_stem),
+ XINT (symbol_int) + 1);
value = intern (buf);
+ SAFE_FREE ();
}
else if (name_table != 0 && name_table[symbol_num])
value = intern (name_table[symbol_num]);
@@ -6483,7 +6488,7 @@
if (NILP (value))
{
- char buf[20];
+ char buf[sizeof "key-" + INT_STRLEN_BOUND (EMACS_INT)];
sprintf (buf, "key-%"pI"d", symbol_num);
value = intern (buf);
}
@@ -10382,19 +10387,21 @@
char *newmessage;
int message_p = push_message ();
int count = SPECPDL_INDEX ();
+ ptrdiff_t newmessage_len, newmessage_alloc;
+ USE_SAFE_ALLOCA;
record_unwind_protect (pop_message_unwind, Qnil);
binding = Fkey_description (bindings, Qnil);
-
- newmessage
- = (char *) alloca (SCHARS (SYMBOL_NAME (function))
- + SBYTES (binding)
- + 100);
- sprintf (newmessage, "You can run the command `%s' with %s",
- SDATA (SYMBOL_NAME (function)),
- SDATA (binding));
+ newmessage_alloc =
+ (sizeof "You can run the command `' with "
+ + SBYTES (SYMBOL_NAME (function)) + SBYTES (binding));
+ SAFE_ALLOCA (newmessage, char *, newmessage_alloc);
+ newmessage_len =
+ esprintf (newmessage, "You can run the command `%s' with %s",
+ SDATA (SYMBOL_NAME (function)),
+ SDATA (binding));
message2 (newmessage,
- strlen (newmessage),
+ newmessage_len,
STRING_MULTIBYTE (binding));
if (NUMBERP (Vsuggest_key_bindings))
waited = sit_for (Vsuggest_key_bindings, 0, 2);
@@ -10404,6 +10411,7 @@
if (!NILP (waited) && message_p)
restore_message ();
+ SAFE_FREE ();
unbind_to (count, Qnil);
}
}
@@ -10633,7 +10641,9 @@
(void)
{
Lisp_Object temp;
- XSETFASTINT (temp, command_loop_level + minibuf_level);
+ /* Wrap around reliably on integer overflow. */
+ EMACS_INT sum = (command_loop_level & INTMASK) + (minibuf_level & INTMASK);
+ XSETINT (temp, sum);
return temp;
}
=== modified file 'src/keymap.c'
--- src/keymap.c 2011-08-14 06:40:45 +0000
+++ src/keymap.c 2011-08-29 15:43:34 +0000
@@ -2143,12 +2143,12 @@
char *
-push_key_description (register unsigned int c, register char *p, int force_multibyte)
+push_key_description (EMACS_INT ch, char *p, int force_multibyte)
{
- unsigned c2;
+ int c, c2;
/* Clear all the meaningless bits above the meta bit. */
- c &= meta_modifier | ~ - meta_modifier;
+ c = ch & (meta_modifier | ~ - meta_modifier);
c2 = c & ~(alt_modifier | ctrl_modifier | hyper_modifier
| meta_modifier | shift_modifier | super_modifier);
@@ -2283,10 +2283,15 @@
{
if (NILP (no_angles))
{
- char *buffer
- = (char *) alloca (SBYTES (SYMBOL_NAME (key)) + 5);
- sprintf (buffer, "<%s>", SDATA (SYMBOL_NAME (key)));
- return build_string (buffer);
+ char *buffer;
+ Lisp_Object result;
+ USE_SAFE_ALLOCA;
+ SAFE_ALLOCA (buffer, char *,
+ sizeof "<>" + SBYTES (SYMBOL_NAME (key)));
+ esprintf (buffer, "<%s>", SDATA (SYMBOL_NAME (key)));
+ result = build_string (buffer);
+ SAFE_FREE ();
+ return result;
}
else
return Fsymbol_name (key);
=== modified file 'src/lisp.h'
--- src/lisp.h 2011-08-14 06:40:45 +0000
+++ src/lisp.h 2011-08-29 15:43:34 +0000
@@ -2895,6 +2895,16 @@
/* Defined in doprnt.c */
extern ptrdiff_t doprnt (char *, ptrdiff_t, const char *, const char *,
va_list);
+extern ptrdiff_t esprintf (char *, char const *, ...)
+ ATTRIBUTE_FORMAT_PRINTF (2, 3);
+extern ptrdiff_t esnprintf (char *, ptrdiff_t, char const *, ...)
+ ATTRIBUTE_FORMAT_PRINTF (3, 4);
+extern ptrdiff_t exprintf (char **, ptrdiff_t *, char const *, ptrdiff_t,
+ char const *, ...)
+ ATTRIBUTE_FORMAT_PRINTF (5, 6);
+extern ptrdiff_t evxprintf (char **, ptrdiff_t *, char const *, ptrdiff_t,
+ char const *, va_list)
+ ATTRIBUTE_FORMAT_PRINTF (5, 0);
/* Defined in lread.c. */
extern Lisp_Object Qvariable_documentation, Qstandard_input;
@@ -3186,7 +3196,7 @@
EXFUN (Feval_minibuffer, 2);
EXFUN (Fread_string, 5);
EXFUN (Fassoc_string, 3);
-extern Lisp_Object get_minibuffer (int);
+extern Lisp_Object get_minibuffer (EMACS_INT);
extern void init_minibuf_once (void);
extern void syms_of_minibuf (void);
@@ -3250,7 +3260,7 @@
extern void init_keyboard (void);
extern void syms_of_keyboard (void);
extern void keys_of_keyboard (void);
-extern char *push_key_description (unsigned int, char *, int);
+extern char *push_key_description (EMACS_INT, char *, int);
/* Defined in indent.c */
=== modified file 'src/lread.c'
--- src/lread.c 2011-08-18 08:41:19 +0000
+++ src/lread.c 2011-08-29 18:55:58 +0000
@@ -4295,14 +4295,20 @@
void
dir_warning (const char *format, Lisp_Object dirname)
{
- char *buffer
- = (char *) alloca (SCHARS (dirname) + strlen (format) + 5);
-
fprintf (stderr, format, SDATA (dirname));
- sprintf (buffer, format, SDATA (dirname));
+
/* Don't log the warning before we've initialized!! */
if (initialized)
- message_dolog (buffer, strlen (buffer), 0, STRING_MULTIBYTE (dirname));
+ {
+ char *buffer;
+ ptrdiff_t message_len;
+ USE_SAFE_ALLOCA;
+ SAFE_ALLOCA (buffer, char *,
+ SBYTES (dirname) + strlen (format) - (sizeof "%s" - 1) + 1);
+ message_len = esprintf (buffer, format, SDATA (dirname));
+ message_dolog (buffer, message_len, 0, STRING_MULTIBYTE (dirname));
+ SAFE_FREE ();
+ }
}
void
=== modified file 'src/macros.c'
--- src/macros.c 2011-07-29 01:00:29 +0000
+++ src/macros.c 2011-08-29 19:07:18 +0000
@@ -35,7 +35,7 @@
This is not bound at each level,
so after an error, it describes the innermost interrupted macro. */
-int executing_kbd_macro_iterations;
+EMACS_INT executing_kbd_macro_iterations;
/* This is the macro that was executing.
This is not bound at each level,
@@ -175,11 +175,11 @@
if (XFASTINT (repeat) == 0)
Fexecute_kbd_macro (KVAR (current_kboard, Vlast_kbd_macro), repeat, loopfunc);
- else
+ else if (XINT (repeat) > 1)
{
XSETINT (repeat, XINT (repeat)-1);
- if (XINT (repeat) > 0)
- Fexecute_kbd_macro (KVAR (current_kboard, Vlast_kbd_macro), repeat, loopfunc);
+ Fexecute_kbd_macro (KVAR (current_kboard, Vlast_kbd_macro),
+ repeat, loopfunc);
}
return Qnil;
}
@@ -302,9 +302,9 @@
Lisp_Object final;
Lisp_Object tem;
int pdlcount = SPECPDL_INDEX ();
- int repeat = 1;
+ EMACS_INT repeat = 1;
struct gcpro gcpro1, gcpro2;
- int success_count = 0;
+ EMACS_INT success_count = 0;
executing_kbd_macro_iterations = 0;
=== modified file 'src/macros.h'
--- src/macros.h 2011-01-25 04:08:28 +0000
+++ src/macros.h 2011-08-29 19:07:18 +0000
@@ -22,7 +22,7 @@
This is not bound at each level,
so after an error, it describes the innermost interrupted macro. */
-extern int executing_kbd_macro_iterations;
+extern EMACS_INT executing_kbd_macro_iterations;
/* This is the macro that was executing.
This is not bound at each level,
@@ -42,4 +42,3 @@
/* Store a character into kbd macro being defined */
extern void store_kbd_macro_char (Lisp_Object);
-
=== modified file 'src/minibuf.c'
--- src/minibuf.c 2011-08-05 02:15:35 +0000
+++ src/minibuf.c 2011-08-29 15:43:34 +0000
@@ -49,7 +49,7 @@
/* Depth in minibuffer invocations. */
-int minibuf_level;
+EMACS_INT minibuf_level;
/* The maximum length of a minibuffer history. */
@@ -772,10 +772,10 @@
used for nonrecursive minibuffer invocations. */
Lisp_Object
-get_minibuffer (int depth)
+get_minibuffer (EMACS_INT depth)
{
Lisp_Object tail, num, buf;
- char name[24];
+ char name[sizeof " *Minibuf-*" + INT_STRLEN_BOUND (EMACS_INT)];
XSETFASTINT (num, depth);
tail = Fnthcdr (num, Vminibuffer_list);
@@ -787,7 +787,7 @@
buf = Fcar (tail);
if (NILP (buf) || NILP (BVAR (XBUFFER (buf), name)))
{
- sprintf (name, " *Minibuf-%d*", depth);
+ sprintf (name, " *Minibuf-%"pI"d*", depth);
buf = Fget_buffer_create (build_string (name));
/* Although the buffer's name starts with a space, undo should be
=== modified file 'src/nsterm.m'
--- src/nsterm.m 2011-08-15 05:30:45 +0000
+++ src/nsterm.m 2011-08-29 19:09:16 +0000
@@ -5316,7 +5316,7 @@
strcpy (old_title, t);
}
size_title = xmalloc (strlen (old_title) + 40);
- sprintf (size_title, "%s — (%d x %d)", old_title, cols, rows);
+ esprintf (size_title, "%s — (%d x %d)", old_title, cols, rows);
[window setTitle: [NSString stringWithUTF8String: size_title]];
[window display];
xfree (size_title);
=== modified file 'src/print.c'
--- src/print.c 2011-07-28 20:23:19 +0000
+++ src/print.c 2011-08-29 19:14:47 +0000
@@ -1016,12 +1016,15 @@
{
width = 0;
do
- width = (width * 10) + (*cp++ - '0');
+ {
+ width = (width * 10) + (*cp++ - '0');
+ if (DBL_DIG < width)
+ goto lose;
+ }
while (*cp >= '0' && *cp <= '9');
/* A precision of zero is valid only for %f. */
- if (width > DBL_DIG
- || (width == 0 && *cp != 'f'))
+ if (width == 0 && *cp != 'f')
goto lose;
}
@@ -1314,7 +1317,9 @@
static void
print_object (Lisp_Object obj, register Lisp_Object printcharfun, int escapeflag)
{
- char buf[40];
+ char buf[max (sizeof "from..to..in " + 2 * INT_STRLEN_BOUND (EMACS_INT),
+ max (sizeof " . #" + INT_STRLEN_BOUND (printmax_t),
+ 40))];
QUIT;
@@ -1614,8 +1619,7 @@
PRINTCHAR ('(');
{
- EMACS_INT print_length;
- int i;
+ printmax_t i, print_length;
Lisp_Object halftail = obj;
/* Negative values of print-length are invalid in CL.
@@ -1623,7 +1627,7 @@
if (NATNUMP (Vprint_length))
print_length = XFASTINT (Vprint_length);
else
- print_length = 0;
+ print_length = TYPE_MAXIMUM (printmax_t);
i = 0;
while (CONSP (obj))
@@ -1634,7 +1638,7 @@
/* Simple but imcomplete way. */
if (i != 0 && EQ (obj, halftail))
{
- sprintf (buf, " . #%d", i / 2);
+ sprintf (buf, " . #%"pMd, i / 2);
strout (buf, -1, -1, printcharfun);
goto end_of_list;
}
@@ -1654,15 +1658,16 @@
}
}
- if (i++)
+ if (i)
PRINTCHAR (' ');
- if (print_length && i > print_length)
+ if (print_length <= i)
{
strout ("...", 3, 3, printcharfun);
goto end_of_list;
}
+ i++;
print_object (XCAR (obj), printcharfun, escapeflag);
obj = XCDR (obj);
@@ -1798,19 +1803,17 @@
PRINTCHAR (' ');
strout (SDATA (SYMBOL_NAME (h->weak)), -1, -1, printcharfun);
PRINTCHAR (' ');
- sprintf (buf, "%ld/%ld", (long) h->count,
- (long) ASIZE (h->next));
+ sprintf (buf, "%"pI"d/%"pI"d", h->count, ASIZE (h->next));
strout (buf, -1, -1, printcharfun);
}
- sprintf (buf, " 0x%lx", (unsigned long) h);
+ sprintf (buf, " %p", h);
strout (buf, -1, -1, printcharfun);
PRINTCHAR ('>');
#endif
/* Implement a readable output, e.g.:
#s(hash-table size 2 test equal data (k1 v1 k2 v2)) */
/* Always print the size. */
- sprintf (buf, "#s(hash-table size %ld",
- (long) ASIZE (h->next));
+ sprintf (buf, "#s(hash-table size %"pI"d", ASIZE (h->next));
strout (buf, -1, -1, printcharfun);
if (!NILP (h->test))
@@ -2038,7 +2041,7 @@
if (MISCP (obj))
sprintf (buf, "(MISC 0x%04x)", (int) XMISCTYPE (obj));
else if (VECTORLIKEP (obj))
- sprintf (buf, "(PVEC 0x%08lx)", (unsigned long) ASIZE (obj));
+ sprintf (buf, "(PVEC 0x%08"pI"x)", ASIZE (obj));
else
sprintf (buf, "(0x%02x)", (int) XTYPE (obj));
strout (buf, -1, -1, printcharfun);
=== modified file 'src/process.c'
--- src/process.c 2011-08-24 21:20:36 +0000
+++ src/process.c 2011-08-29 19:46:15 +0000
@@ -616,8 +616,8 @@
{
register Lisp_Object val, tem, name1;
register struct Lisp_Process *p;
- char suffix[10];
- register int i;
+ char suffix[sizeof "<>" + INT_STRLEN_BOUND (printmax_t)];
+ printmax_t i;
p = allocate_process ();
@@ -651,7 +651,7 @@
{
tem = Fget_process (name1);
if (NILP (tem)) break;
- sprintf (suffix, "<%d>", i);
+ sprintf (suffix, "<%"pMd">", i);
name1 = concat2 (name, build_string (suffix));
}
name = name1;
=== modified file 'src/term.c'
--- src/term.c 2011-08-30 17:32:44 +0000
+++ src/term.c 2011-08-30 21:16:49 +0000
@@ -1817,7 +1817,7 @@
{
int face_id;
int len;
- char buf[9];
+ char buf[sizeof "\\x" + max (6, (sizeof it->c * CHAR_BIT + 3) / 4)];
char const *str = " ";
/* Get a face ID for the glyph by utilizing a cache (the same way as
=== modified file 'src/window.h'
--- src/window.h 2011-07-02 10:36:48 +0000
+++ src/window.h 2011-08-29 15:43:34 +0000
@@ -847,11 +847,11 @@
/* Depth in recursive edits. */
-extern int command_loop_level;
+extern EMACS_INT command_loop_level;
/* Depth in minibuffer invocations. */
-extern int minibuf_level;
+extern EMACS_INT minibuf_level;
/* true if we should redraw the mode lines on the next redisplay. */
=== modified file 'src/xfaces.c'
--- src/xfaces.c 2011-08-19 14:28:36 +0000
+++ src/xfaces.c 2011-08-29 19:58:56 +0000
@@ -3549,6 +3549,8 @@
rdb != NULL))
{
char line[512];
+ char *buf = line;
+ ptrdiff_t bufsize = sizeof line;
Lisp_Object lface = lface_from_face_name (f, Qmenu, 1);
struct face *face = FACE_FROM_ID (f, MENU_FACE_ID);
const char *myname = SSDATA (Vx_resource_name);
@@ -3561,24 +3563,25 @@
if (STRINGP (LFACE_FOREGROUND (lface)))
{
- sprintf (line, "%s.%s*foreground: %s",
- myname, popup_path,
- SDATA (LFACE_FOREGROUND (lface)));
+ exprintf (&buf, &bufsize, line, -1, "%s.%s*foreground: %s",
+ myname, popup_path,
+ SDATA (LFACE_FOREGROUND (lface)));
XrmPutLineResource (&rdb, line);
- sprintf (line, "%s.pane.menubar*foreground: %s",
- myname, SDATA (LFACE_FOREGROUND (lface)));
+ exprintf (&buf, &bufsize, line, -1, "%s.pane.menubar*foreground: %s",
+ myname, SDATA (LFACE_FOREGROUND (lface)));
XrmPutLineResource (&rdb, line);
changed_p = 1;
}
if (STRINGP (LFACE_BACKGROUND (lface)))
{
- sprintf (line, "%s.%s*background: %s",
- myname, popup_path,
- SDATA (LFACE_BACKGROUND (lface)));
+ exprintf (&buf, &bufsize, line, -1, "%s.%s*background: %s",
+ myname, popup_path,
+ SDATA (LFACE_BACKGROUND (lface)));
XrmPutLineResource (&rdb, line);
- sprintf (line, "%s.pane.menubar*background: %s",
- myname, SDATA (LFACE_BACKGROUND (lface)));
+
+ exprintf (&buf, &bufsize, line, -1, "%s.pane.menubar*background: %s",
+ myname, SDATA (LFACE_BACKGROUND (lface)));
XrmPutLineResource (&rdb, line);
changed_p = 1;
}
@@ -3616,11 +3619,12 @@
#else
char *fontsetname = SSDATA (xlfd);
#endif
- sprintf (line, "%s.pane.menubar*font%s: %s",
- myname, suffix, fontsetname);
+ exprintf (&buf, &bufsize, line, -1, "%s.pane.menubar*font%s: %s",
+ myname, suffix, fontsetname);
XrmPutLineResource (&rdb, line);
- sprintf (line, "%s.%s*font%s: %s",
- myname, popup_path, suffix, fontsetname);
+
+ exprintf (&buf, &bufsize, line, -1, "%s.%s*font%s: %s",
+ myname, popup_path, suffix, fontsetname);
XrmPutLineResource (&rdb, line);
changed_p = 1;
if (fontsetname != SSDATA (xlfd))
@@ -3630,6 +3634,9 @@
if (changed_p && f->output_data.x->menubar_widget)
free_frame_menubar (f);
+
+ if (buf != line)
+ xfree (buf);
}
}
=== modified file 'src/xfns.c'
--- src/xfns.c 2011-08-05 02:15:35 +0000
+++ src/xfns.c 2011-08-29 19:59:51 +0000
@@ -2440,7 +2440,7 @@
/* Do some needed geometry management. */
{
ptrdiff_t len;
- char *tem, shell_position[32];
+ char *tem, shell_position[sizeof "=x++" + 4 * INT_STRLEN_BOUND (int)];
Arg gal[10];
int gac = 0;
int extra_borders = 0;
=== modified file 'src/xterm.c'
--- src/xterm.c 2011-08-05 02:19:34 +0000
+++ src/xterm.c 2011-08-29 20:03:30 +0000
@@ -7900,7 +7900,8 @@
{
char buf[256];
- sprintf (buf, "Connection lost to X server `%s'", DisplayString (display));
+ snprintf (buf, sizeof buf, "Connection lost to X server `%s'",
+ DisplayString (display));
x_connection_closed (display, buf);
return 0;
}
=== modified file 'src/xterm.h'
--- src/xterm.h 2011-07-07 02:24:56 +0000
+++ src/xterm.h 2011-08-29 20:04:46 +0000
@@ -955,7 +955,8 @@
extern int x_text_icon (struct frame *, const char *);
extern int x_bitmap_icon (struct frame *, Lisp_Object);
extern void x_catch_errors (Display *);
-extern void x_check_errors (Display *, const char *);
+extern void x_check_errors (Display *, const char *)
+ ATTRIBUTE_FORMAT_PRINTF (2, 0);
extern int x_had_errors_p (Display *);
extern int x_catching_errors (void);
extern void x_uncatch_errors (void);
^ permalink raw reply [flat|nested] 8+ messages in thread* bug#9412: sprintf-related integer and memory overflow issues
2011-08-30 22:42 bug#9412: sprintf-related integer and memory overflow issues Paul Eggert
@ 2011-08-31 2:08 ` Chong Yidong
2011-08-31 6:03 ` Paul Eggert
0 siblings, 1 reply; 8+ messages in thread
From: Chong Yidong @ 2011-08-31 2:08 UTC (permalink / raw)
To: Paul Eggert; +Cc: 9412
Paul Eggert <eggert@cs.ucla.edu> writes:
> Here's a patch to the Emacs trunk to fix some sprintf-related integer
> and memory overflow issues in Emacs proper. These bugs can cause the
> wrong integer to be displayed, or a buffer overrun in sprintf output,
> that sort of thing. Almost all the bugs can occur independently of
> whether --with-wide-int is used. The bugs range from unlikely to
> extremely unlikely in normal use (otherwise they would have been fixed
> already....). The patch is (I hope) routine. I plan to install this
> patch after some more internal testing.
I don't much like the idea of using custom functions like esprintf and
esnprintf. They make the code much less clear.
Also, I seem to recall that the reason we don't use snprintf is that
it's not available on all the platforms that Emacs supports.
^ permalink raw reply [flat|nested] 8+ messages in thread
* bug#9412: sprintf-related integer and memory overflow issues
2011-08-31 2:08 ` Chong Yidong
@ 2011-08-31 6:03 ` Paul Eggert
2011-08-31 16:14 ` Chong Yidong
2011-08-31 16:53 ` Dan Nicolaescu
0 siblings, 2 replies; 8+ messages in thread
From: Paul Eggert @ 2011-08-31 6:03 UTC (permalink / raw)
To: Chong Yidong; +Cc: 9412
On 08/30/11 19:08, Chong Yidong wrote:
> I don't much like the idea of using custom functions like esprintf and
> esnprintf. They make the code much less clear.
In code that is formatting buffers or strings that might be longer
than 2 GiB, we cannot use sprintf etc. We need to fix these issues
somehow. If there's a better way than esprintf etc. please let me know.
Anyway, I don't quite follow why using esprintf etc. makes the calling
code much less clear. esprintf is like sprintf, except that on 64-bit
hosts esprintf doesn't have sprintf's 2 GiB limit. If the use of
esprintf is unclear, then surely the use of sprintf is just as unclear.
> I seem to recall that the reason we don't use snprintf is that
> it's not available on all the platforms that Emacs supports.
Although that used to be true, I expect that platforms lacking
snprintf (e.g., Solaris 2.5.1, IRIX 5.3, OSF/1 4.0) are no longer of
practical importance as Emacs porting targets.
That being said, it's easy to allay this concern, by using esnprintf
instead of snprintf in all areas of the code that might run on
ancient platforms. The following further patch does this.
====
Avoid the use of snprintf.
* font.c (APPEND_SNPRINTF): Remove.
(font_unparse_xlfd):
* xterm.c (x_io_error_quitter):
Use esnprintf, not snprintf. That way, we don't have to worry
about porting to ancient platforms that lack snprintf.
(x_term_init): Use sprintf, not snprintf.
=== modified file 'src/font.c'
--- src/font.c 2011-08-29 20:57:42 +0000
+++ src/font.c 2011-08-31 05:50:10 +0000
@@ -1285,14 +1285,14 @@
}
else
f[XLFD_AVGWIDTH_INDEX] = "*";
- len = snprintf (name, nbytes, "-%s-%s-%s-%s-%s-%s-%s-%s-%s-%s-%s",
- f[XLFD_FOUNDRY_INDEX], f[XLFD_FAMILY_INDEX],
- f[XLFD_WEIGHT_INDEX], f[XLFD_SLANT_INDEX],
- f[XLFD_SWIDTH_INDEX], f[XLFD_ADSTYLE_INDEX],
- f[XLFD_PIXEL_INDEX], f[XLFD_RESX_INDEX],
- f[XLFD_SPACING_INDEX], f[XLFD_AVGWIDTH_INDEX],
- f[XLFD_REGISTRY_INDEX]);
- return len < nbytes ? len : -1;
+ len = esnprintf (name, nbytes, "-%s-%s-%s-%s-%s-%s-%s-%s-%s-%s-%s",
+ f[XLFD_FOUNDRY_INDEX], f[XLFD_FAMILY_INDEX],
+ f[XLFD_WEIGHT_INDEX], f[XLFD_SLANT_INDEX],
+ f[XLFD_SWIDTH_INDEX], f[XLFD_ADSTYLE_INDEX],
+ f[XLFD_PIXEL_INDEX], f[XLFD_RESX_INDEX],
+ f[XLFD_SPACING_INDEX], f[XLFD_AVGWIDTH_INDEX],
+ f[XLFD_REGISTRY_INDEX]);
+ return len == nbytes - 1 ? -1 : len;
}
/* Parse NAME (null terminated) and store information in FONT
@@ -1592,39 +1592,32 @@
p = name;
lim = name + nbytes;
-# define APPEND_SNPRINTF(args) \
- do { \
- int len = snprintf args; \
- if (! (0 <= len && len < lim - p)) \
- return -1; \
- p += len; \
- } while (0)
if (! NILP (family))
- APPEND_SNPRINTF ((p, lim - p, "%s", SSDATA (family)));
+ p += esnprintf (p, lim - p, "%s", SSDATA (family));
if (point_size > 0)
- APPEND_SNPRINTF ((p, lim - p, "-%d" + (p == name), point_size));
+ p += esnprintf (p, lim - p, "-%d" + (p == name), point_size);
else if (pixel_size > 0)
- APPEND_SNPRINTF ((p, lim - p, ":pixelsize=%d", pixel_size));
+ p += esnprintf (p, lim - p, ":pixelsize=%d", pixel_size);
if (! NILP (AREF (font, FONT_FOUNDRY_INDEX)))
- APPEND_SNPRINTF ((p, lim - p, ":foundry=%s",
- SSDATA (SYMBOL_NAME (AREF (font,
- FONT_FOUNDRY_INDEX)))));
+ p += esnprintf (p, lim - p, ":foundry=%s",
+ SSDATA (SYMBOL_NAME (AREF (font,
+ FONT_FOUNDRY_INDEX))));
for (i = 0; i < 3; i++)
if (! NILP (styles[i]))
- APPEND_SNPRINTF ((p, lim - p, ":%s=%s", style_names[i],
- SSDATA (SYMBOL_NAME (styles[i]))));
+ p += esnprintf (p, lim - p, ":%s=%s", style_names[i],
+ SSDATA (SYMBOL_NAME (styles[i])));
if (INTEGERP (AREF (font, FONT_DPI_INDEX)))
- APPEND_SNPRINTF ((p, lim - p, ":dpi=%"pI"d",
- XINT (AREF (font, FONT_DPI_INDEX))));
+ p += esnprintf (p, lim - p, ":dpi=%"pI"d",
+ XINT (AREF (font, FONT_DPI_INDEX)));
if (INTEGERP (AREF (font, FONT_SPACING_INDEX)))
- APPEND_SNPRINTF ((p, lim - p, ":spacing=%"pI"d",
- XINT (AREF (font, FONT_SPACING_INDEX))));
+ p += esnprintf (p, lim - p, ":spacing=%"pI"d",
+ XINT (AREF (font, FONT_SPACING_INDEX)));
if (INTEGERP (AREF (font, FONT_AVGWIDTH_INDEX)))
- APPEND_SNPRINTF ((p, lim - p,
- (XINT (AREF (font, FONT_AVGWIDTH_INDEX)) == 0
- ? ":scalable=true"
- : ":scalable=false")));
- return (p - name);
+ p += esnprintf (p, lim - p,
+ (XINT (AREF (font, FONT_AVGWIDTH_INDEX)) == 0
+ ? ":scalable=true"
+ : ":scalable=false"));
+ return lim - p == 1 ? -1 : p - name;
}
/* Parse NAME (null terminated) and store information in FONT
=== modified file 'src/xterm.c'
--- src/xterm.c 2011-08-29 20:03:30 +0000
+++ src/xterm.c 2011-08-31 05:24:36 +0000
@@ -7900,8 +7900,8 @@
{
char buf[256];
- snprintf (buf, sizeof buf, "Connection lost to X server `%s'",
- DisplayString (display));
+ esnprintf (buf, sizeof buf, "Connection lost to X server `%s'",
+ DisplayString (display));
x_connection_closed (display, buf);
return 0;
}
@@ -10278,8 +10278,8 @@
atom_names[i] = (char *) atom_refs[i].name;
/* Build _XSETTINGS_SN atom name */
- snprintf (xsettings_atom_name, sizeof (xsettings_atom_name),
- "_XSETTINGS_S%d", XScreenNumberOfScreen (dpyinfo->screen));
+ sprintf (xsettings_atom_name,
+ "_XSETTINGS_S%d", XScreenNumberOfScreen (dpyinfo->screen));
atom_names[i] = xsettings_atom_name;
XInternAtoms (dpyinfo->display, atom_names, total_atom_count,
^ permalink raw reply [flat|nested] 8+ messages in thread* bug#9412: sprintf-related integer and memory overflow issues
2011-08-31 6:03 ` Paul Eggert
@ 2011-08-31 16:14 ` Chong Yidong
2011-08-31 22:21 ` Paul Eggert
2011-08-31 16:53 ` Dan Nicolaescu
1 sibling, 1 reply; 8+ messages in thread
From: Chong Yidong @ 2011-08-31 16:14 UTC (permalink / raw)
To: Paul Eggert; +Cc: 9412
Paul Eggert <eggert@cs.ucla.edu> writes:
> In code that is formatting buffers or strings that might be longer
> than 2 GiB, we cannot use sprintf etc. We need to fix these issues
> somehow. If there's a better way than esprintf etc. please let me
> know.
>
> Anyway, I don't quite follow why using esprintf etc. makes the calling
> code much less clear. esprintf is like sprintf, except that on 64-bit
> hosts esprintf doesn't have sprintf's 2 GiB limit. If the use of
> esprintf is unclear, then surely the use of sprintf is just as
> unclear.
sprintf (and snprintf) is a well-known function; when someone comes
across it in the code, it's not necessary to look it up to know what
it's doing.
But I'm not clear on this issue of sprintf etc being restricted to 2GB.
Could you explain further? Surely such a limitation is a bug in the C
library, not Emacs? If so, it should be fixed there, not in Emacs.
> Although that used to be true, I expect that platforms lacking
> snprintf (e.g., Solaris 2.5.1, IRIX 5.3, OSF/1 4.0) are no longer of
> practical importance as Emacs porting targets.
>
> That being said, it's easy to allay this concern, by using esnprintf
> instead of snprintf in all areas of the code that might run on ancient
> platforms. The following further patch does this.
I think we should add a stub for snprintf in sysdep.c for the
!HAVE_SNPRINTF case (which will need configure to set up HAVE_SNPRINTF).
^ permalink raw reply [flat|nested] 8+ messages in thread
* bug#9412: sprintf-related integer and memory overflow issues
2011-08-31 16:14 ` Chong Yidong
@ 2011-08-31 22:21 ` Paul Eggert
2011-09-01 2:42 ` Chong Yidong
0 siblings, 1 reply; 8+ messages in thread
From: Paul Eggert @ 2011-08-31 22:21 UTC (permalink / raw)
To: Chong Yidong; +Cc: 9412
On 08/31/11 09:14, Chong Yidong wrote:
> I'm not clear on this issue of sprintf etc being restricted to 2GB.
> Could you explain further?
sprintf returns an 'int' counting the number of bytes it outputs.
If this count would exceed INT_MAX, sprintf returns -1 and sets errno.
sprintf therefore cannot be used to create a string that might be
longer than INT_MAX bytes. On typical 64-bit hosts, this is an
arbitrary 2 GiB limit, which Emacs shouldn't have.
> Surely such a limitation is a bug in the C library, not Emacs?
> If so, it should be fixed there, not in Emacs.
I agree, but unfortunately the problem is inherent to sprintf's type
signature, which has been stable for many years and is not slated to
be improved or extended even in C1X. It would take decades before we
could assume any such fix would be present in the C library.
> sprintf (and snprintf) is a well-known function; when someone comes
> across it in the code, it's not necessary to look it up to know what
> it's doing.
True, but surely it's not too much to ask Emacs developers to remember
that esprintf is like sprintf, except without the 2 GiB limit. Emacs
developers already deal with similar printf-like functions ('message',
'error', etc.) and this sort of thing is nothing new.
I wouldn't be suggesting esprintf if it weren't for the 2 GiB limit.
> I think we should add a stub for snprintf in sysdep.c for the
> !HAVE_SNPRINTF case (which will need configure to set up HAVE_SNPRINTF).
Sure, that's easily done, with the following additional patch.
I'll CC: this to Eli since it adds a #define to nt/config.nt.
=== modified file 'ChangeLog'
--- ChangeLog 2011-08-30 20:46:59 +0000
+++ ChangeLog 2011-08-31 22:18:16 +0000
@@ -1,3 +1,7 @@
+2011-08-31 Paul Eggert <eggert@cs.ucla.edu>
+
+ * configure.in (snprintf): New check.
+
2011-08-30 Paul Eggert <eggert@cs.ucla.edu>
* configure.in (opsys): Change pattern to *-*-linux*
=== modified file 'configure.in'
--- configure.in 2011-08-30 20:46:59 +0000
+++ configure.in 2011-08-31 22:18:16 +0000
@@ -3071,6 +3071,8 @@
AC_FUNC_FORK
+AC_CHECK_FUNCS(snprintf)
+
dnl Adapted from Haible's version.
AC_CACHE_CHECK([for nl_langinfo and CODESET], emacs_cv_langinfo_codeset,
[AC_LINK_IFELSE([AC_LANG_PROGRAM([[#include <langinfo.h>]],
=== modified file 'nt/ChangeLog'
--- nt/ChangeLog 2011-07-28 00:48:01 +0000
+++ nt/ChangeLog 2011-08-31 22:18:16 +0000
@@ -1,3 +1,7 @@
+2011-08-31 Paul Eggert <eggert@cs.ucla.edu>
+
+ * config.nt (HAVE_SNPRINTF): New macro.
+
2011-07-28 Paul Eggert <eggert@cs.ucla.edu>
Assume freestanding C89 headers, string.h, stdlib.h.
=== modified file 'nt/config.nt'
--- nt/config.nt 2011-07-07 01:32:56 +0000
+++ nt/config.nt 2011-08-31 22:18:16 +0000
@@ -240,6 +240,7 @@
#define HAVE_SETSOCKOPT 1
#define HAVE_GETSOCKNAME 1
#define HAVE_GETPEERNAME 1
+#define HAVE_SNPRINTF 1
#define HAVE_LANGINFO_CODESET 1
/* Local (unix) sockets are not supported. */
#undef HAVE_SYS_UN_H
=== modified file 'src/ChangeLog'
--- src/ChangeLog 2011-08-31 20:02:51 +0000
+++ src/ChangeLog 2011-08-31 22:18:16 +0000
@@ -90,6 +90,8 @@
* process.c (make_process): Use printmax_t, not int, to format
process-name gensyms.
+ * sysdep.c (snprintf) [! HAVE_SNPRINTF]: New function.
+
* term.c (produce_glyphless_glyph): Make sprintf buffer a bit bigger
to avoid potential buffer overrun.
=== modified file 'src/sysdep.c'
--- src/sysdep.c 2011-07-29 01:16:54 +0000
+++ src/sysdep.c 2011-08-31 22:18:16 +0000
@@ -1811,6 +1811,45 @@
}
#endif /* not WINDOWSNT */
#endif /* ! HAVE_STRERROR */
+
+#ifndef HAVE_SNPRINTF
+/* Approximate snprintf as best we can on ancient hosts that lack it. */
+int
+snprintf (char *buf, size_t bufsize, char const *format, ...)
+{
+ ptrdiff_t size = min (bufsize, PTRDIFF_MAX);
+ ptrdiff_t nbytes = size - 1;
+ va_list ap;
+
+ if (size)
+ {
+ va_start (ap, format);
+ nbytes = doprnt (buf, size, format, 0, ap);
+ va_end (ap);
+ }
+
+ if (nbytes == size - 1)
+ {
+ /* Calculate the length of the string that would have been created
+ had the buffer been large enough. */
+ char stackbuf[4000];
+ char *b = stackbuf;
+ ptrdiff_t bsize = sizeof stackbuf;
+ va_start (ap, format);
+ nbytes = evxprintf (&b, &bsize, stackbuf, -1, format, ap);
+ va_end (ap);
+ if (b != stackbuf)
+ xfree (b);
+ }
+
+ if (INT_MAX < nbytes)
+ {
+ errno = EOVERFLOW;
+ return -1;
+ }
+ return nbytes;
+}
+#endif
\f
int
emacs_open (const char *path, int oflag, int mode)
^ permalink raw reply [flat|nested] 8+ messages in thread* bug#9412: sprintf-related integer and memory overflow issues
2011-08-31 22:21 ` Paul Eggert
@ 2011-09-01 2:42 ` Chong Yidong
2011-09-01 5:44 ` Paul Eggert
0 siblings, 1 reply; 8+ messages in thread
From: Chong Yidong @ 2011-09-01 2:42 UTC (permalink / raw)
To: Paul Eggert; +Cc: 9412
Paul Eggert <eggert@cs.ucla.edu> writes:
>> Surely such a limitation is a bug in the C library, not Emacs?
>> If so, it should be fixed there, not in Emacs.
>
> I agree, but unfortunately the problem is inherent to sprintf's type
> signature, which has been stable for many years and is not slated to
> be improved or extended even in C1X. It would take decades before we
> could assume any such fix would be present in the C library.
>
>> I think we should add a stub for snprintf in sysdep.c for the
>> !HAVE_SNPRINTF case (which will need configure to set up HAVE_SNPRINTF).
>
> Sure, that's easily done, with the following additional patch.
> I'll CC: this to Eli since it adds a #define to nt/config.nt.
With this, the parts of the patch involving using snprintf look good.
But I would prefer to defer the esnprintf stuff till after 24.1.
The changes to font_unparse_xlfd and font_unparse_fcname don't look as
straightforward as the rest. That code is somewhat fragile, so I
suggest double checking those changes (or, better yet, writing a test
case).
^ permalink raw reply [flat|nested] 8+ messages in thread
* bug#9412: sprintf-related integer and memory overflow issues
2011-09-01 2:42 ` Chong Yidong
@ 2011-09-01 5:44 ` Paul Eggert
0 siblings, 0 replies; 8+ messages in thread
From: Paul Eggert @ 2011-09-01 5:44 UTC (permalink / raw)
To: Chong Yidong; +Cc: 9412
On 08/31/11 19:42, Chong Yidong wrote:
> With this, the parts of the patch involving using snprintf look good.
> But I would prefer to defer the esnprintf stuff till after 24.1.
OK, I'll remove esnprintf from the patch before installing.
> The changes to font_unparse_xlfd and font_unparse_fcname don't look as
> straightforward as the rest. That code is somewhat fragile, so I
> suggest double checking those changes (or, better yet, writing a test
> case).
I'll do that too.
^ permalink raw reply [flat|nested] 8+ messages in thread
* bug#9412: sprintf-related integer and memory overflow issues
2011-08-31 6:03 ` Paul Eggert
2011-08-31 16:14 ` Chong Yidong
@ 2011-08-31 16:53 ` Dan Nicolaescu
1 sibling, 0 replies; 8+ messages in thread
From: Dan Nicolaescu @ 2011-08-31 16:53 UTC (permalink / raw)
To: Paul Eggert; +Cc: 9412, Chong Yidong
Paul Eggert <eggert@cs.ucla.edu> writes:
> On 08/30/11 19:08, Chong Yidong wrote:
>
>> I seem to recall that the reason we don't use snprintf is that
>> it's not available on all the platforms that Emacs supports.
>
> Although that used to be true, I expect that platforms lacking
> snprintf (e.g., Solaris 2.5.1, IRIX 5.3, OSF/1 4.0) are no longer of
> practical importance as Emacs porting targets.
FWIW, we removed support in emacs for those 3 OSes a few years ago.
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2011-09-01 5:44 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-08-30 22:42 bug#9412: sprintf-related integer and memory overflow issues Paul Eggert
2011-08-31 2:08 ` Chong Yidong
2011-08-31 6:03 ` Paul Eggert
2011-08-31 16:14 ` Chong Yidong
2011-08-31 22:21 ` Paul Eggert
2011-09-01 2:42 ` Chong Yidong
2011-09-01 5:44 ` Paul Eggert
2011-08-31 16:53 ` Dan Nicolaescu
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
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).