From 8666506ecd6b1a90c7c66fb4b6051e90fba20c30 Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Thu, 1 Sep 2011 07:44:49 -0700 Subject: [PATCH] * src/doprnt.c (esnprintf): Remove. All uses removed. Suggested by Chong Yidong in . --- src/ChangeLog | 11 +++--- src/dbusbind.c | 42 ++++++++++++++-------- src/dispnew.c | 20 +++++------ src/doprnt.c | 21 ----------- src/font.c | 94 ++++++++++++++++++++++++++++++++++++-------------- src/lisp.h | 2 -- src/xterm.c | 8 ++--- 7 files changed, 115 insertions(+), 83 deletions(-) diff --git a/src/ChangeLog b/src/ChangeLog index 0ba2df42186..94ba3a040ca 100644 --- a/src/ChangeLog +++ b/src/ChangeLog @@ -1,9 +1,9 @@ -2011-08-31 Paul Eggert +2011-09-01 Paul Eggert sprintf-related integer and memory overflow issues (Bug#9412). * doprnt.c (doprnt): Support printing ptrdiff_t and intmax_t values. - (esprintf, esnprintf, exprintf, evxprintf): New functions. + (esprintf, 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 @@ -17,7 +17,7 @@ * 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. + (esprintf, exprintf, evxprintf): New decls. * window.h (command_loop_level, minibuf_level): Reflect API changes. * dbusbind.c (signature_cat): New function. @@ -43,7 +43,7 @@ * font.c: Include , 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 esnprintf to count + 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. (font_unparse_fcname): Use it to avoid sprintf buffer overrun. @@ -102,9 +102,6 @@ to avoid potential buffer overrun. * xterm.c (x_io_error_quitter): Don't overrun sprintf buffer. - (x_term_init): Use sprintf, not snprintf, so that we need not - worry about ancient hosts that lack snprintf. The buffer cannot - possibly be overrun, so this is safe. * xterm.h (x_check_errors): Add ATTRIBUTE_FORMAT_PRINTF. diff --git a/src/dbusbind.c b/src/dbusbind.c index fd9a43aaf86..8dac2a6249f 100644 --- a/src/dbusbind.c +++ b/src/dbusbind.c @@ -284,6 +284,7 @@ xd_signature (char *signature, unsigned int dtype, unsigned int parent_type, Lis unsigned int subtype; Lisp_Object elt; char const *subsig; + int subsiglen; char x[DBUS_MAXIMUM_SIGNATURE_LENGTH]; elt = object; @@ -365,9 +366,9 @@ xd_signature (char *signature, unsigned int dtype, unsigned int parent_type, Lis elt = CDR_SAFE (XD_NEXT_VALUE (elt)); } - if (esnprintf (signature, DBUS_MAXIMUM_SIGNATURE_LENGTH, - "%c%s", dtype, subsig) - == DBUS_MAXIMUM_SIGNATURE_LENGTH - 1) + subsiglen = snprintf (signature, DBUS_MAXIMUM_SIGNATURE_LENGTH, + "%c%s", dtype, subsig); + if (! (0 <= subsiglen && subsiglen < DBUS_MAXIMUM_SIGNATURE_LENGTH)) string_overflow (); break; @@ -2088,32 +2089,45 @@ usage: (dbus-register-signal BUS SERVICE PATH INTERFACE SIGNAL HANDLER &rest ARG connection = xd_initialize (bus, TRUE); /* Create a rule to receive related signals. */ - rulelen = esnprintf (rule, sizeof rule, - "type='signal',interface='%s',member='%s'", - SDATA (interface), - SDATA (signal)); + rulelen = snprintf (rule, sizeof rule, + "type='signal',interface='%s',member='%s'", + SDATA (interface), + SDATA (signal)); + if (! (0 <= rulelen && rulelen < sizeof rule)) + string_overflow (); /* Add unique name and path to the rule if they are non-nil. */ if (!NILP (uname)) - rulelen += esnprintf (rule + rulelen, sizeof rule - rulelen, + { + int len = snprintf (rule + rulelen, sizeof rule - rulelen, ",sender='%s'", SDATA (uname)); + if (! (0 <= len && len < sizeof rule - rulelen)) + string_overflow (); + rulelen += len; + } if (!NILP (path)) - rulelen += esnprintf (rule + rulelen, sizeof rule - rulelen, + { + int len = snprintf (rule + rulelen, sizeof rule - rulelen, ",path='%s'", SDATA (path)); + if (! (0 <= len && len < sizeof rule - rulelen)) + string_overflow (); + rulelen += len; + } /* Add arguments to the rule if they are non-nil. */ for (i = 6; i < nargs; ++i) if (!NILP (args[i])) { + int len; CHECK_STRING (args[i]); - rulelen += esnprintf (rule + rulelen, sizeof rule - rulelen, - ",arg%"pD"d='%s'", i - 6, SDATA (args[i])); + len = snprintf (rule + rulelen, sizeof rule - rulelen, + ",arg%"pD"d='%s'", i - 6, SDATA (args[i])); + if (! (0 <= len && len < sizeof rule - rulelen)) + string_overflow (); + rulelen += len; } - if (rulelen == sizeof rule - 1) - string_overflow (); - /* Add the rule to the bus. */ dbus_error_init (&derror); dbus_bus_add_match (connection, rule, &derror); diff --git a/src/dispnew.c b/src/dispnew.c index 0cc888b4b7a..5c28d014819 100644 --- a/src/dispnew.c +++ b/src/dispnew.c @@ -272,16 +272,16 @@ add_window_display_history (struct window *w, const char *msg, int paused_p) buf = redisplay_history[history_idx].trace; ++history_idx; - 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); + snprintf (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); } diff --git a/src/doprnt.c b/src/doprnt.c index dae1dab04d7..638fa4d6312 100644 --- a/src/doprnt.c +++ b/src/doprnt.c @@ -486,27 +486,6 @@ esprintf (char *buf, char const *format, ...) 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 diff --git a/src/font.c b/src/font.c index a5b873aea51..34cacb37ce4 100644 --- a/src/font.c +++ b/src/font.c @@ -1285,14 +1285,14 @@ font_unparse_xlfd (Lisp_Object font, int pixel_size, char *name, int nbytes) } else f[XLFD_AVGWIDTH_INDEX] = "*"; - 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; + 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 @@ -1593,31 +1593,75 @@ font_unparse_fcname (Lisp_Object font, int pixel_size, char *name, int nbytes) p = name; lim = name + nbytes; if (! NILP (family)) - p += esnprintf (p, lim - p, "%s", SSDATA (family)); + { + int len = snprintf (p, lim - p, "%s", SSDATA (family)); + if (! (0 <= len && len < lim - p)) + return -1; + p += len; + } if (point_size > 0) - p += esnprintf (p, lim - p, "-%d" + (p == name), point_size); + { + int len = snprintf (p, lim - p, "-%d" + (p == name), point_size); + if (! (0 <= len && len < lim - p)) + return -1; + p += len; + } else if (pixel_size > 0) - p += esnprintf (p, lim - p, ":pixelsize=%d", pixel_size); + { + int len = snprintf (p, lim - p, ":pixelsize=%d", pixel_size); + if (! (0 <= len && len < lim - p)) + return -1; + p += len; + } if (! NILP (AREF (font, FONT_FOUNDRY_INDEX))) - p += esnprintf (p, lim - p, ":foundry=%s", - SSDATA (SYMBOL_NAME (AREF (font, - FONT_FOUNDRY_INDEX)))); + { + int len = snprintf (p, lim - p, ":foundry=%s", + SSDATA (SYMBOL_NAME (AREF (font, + FONT_FOUNDRY_INDEX)))); + if (! (0 <= len && len < lim - p)) + return -1; + p += len; + } for (i = 0; i < 3; i++) if (! NILP (styles[i])) - p += esnprintf (p, lim - p, ":%s=%s", style_names[i], - SSDATA (SYMBOL_NAME (styles[i]))); + { + int len = snprintf (p, lim - p, ":%s=%s", style_names[i], + SSDATA (SYMBOL_NAME (styles[i]))); + if (! (0 <= len && len < lim - p)) + return -1; + p += len; + } + if (INTEGERP (AREF (font, FONT_DPI_INDEX))) - p += esnprintf (p, lim - p, ":dpi=%"pI"d", - XINT (AREF (font, FONT_DPI_INDEX))); + { + int len = snprintf (p, lim - p, ":dpi=%"pI"d", + XINT (AREF (font, FONT_DPI_INDEX))); + if (! (0 <= len && len < lim - p)) + return -1; + p += len; + } + if (INTEGERP (AREF (font, FONT_SPACING_INDEX))) - p += esnprintf (p, lim - p, ":spacing=%"pI"d", - XINT (AREF (font, FONT_SPACING_INDEX))); + { + int len = snprintf (p, lim - p, ":spacing=%"pI"d", + XINT (AREF (font, FONT_SPACING_INDEX))); + if (! (0 <= len && len < lim - p)) + return -1; + p += len; + } + if (INTEGERP (AREF (font, FONT_AVGWIDTH_INDEX))) - p += esnprintf (p, lim - p, - (XINT (AREF (font, FONT_AVGWIDTH_INDEX)) == 0 - ? ":scalable=true" - : ":scalable=false")); - return lim - p == 1 ? -1 : p - name; + { + int len = snprintf (p, lim - p, + (XINT (AREF (font, FONT_AVGWIDTH_INDEX)) == 0 + ? ":scalable=true" + : ":scalable=false")); + if (! (0 <= len && len < lim - p)) + return -1; + p += len; + } + + return (p - name); } /* Parse NAME (null terminated) and store information in FONT diff --git a/src/lisp.h b/src/lisp.h index 2f6ec38f228..5c84bb8e06e 100644 --- a/src/lisp.h +++ b/src/lisp.h @@ -2897,8 +2897,6 @@ 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); diff --git a/src/xterm.c b/src/xterm.c index 72e9f2b2236..86393cf411f 100644 --- a/src/xterm.c +++ b/src/xterm.c @@ -7900,8 +7900,8 @@ x_io_error_quitter (Display *display) { char buf[256]; - esnprintf (buf, sizeof 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; } @@ -10278,8 +10278,8 @@ x_term_init (Lisp_Object display_name, char *xrm_option, char *resource_name) atom_names[i] = (char *) atom_refs[i].name; /* Build _XSETTINGS_SN atom name */ - sprintf (xsettings_atom_name, - "_XSETTINGS_S%d", XScreenNumberOfScreen (dpyinfo->screen)); + snprintf (xsettings_atom_name, sizeof (xsettings_atom_name), + "_XSETTINGS_S%d", XScreenNumberOfScreen (dpyinfo->screen)); atom_names[i] = xsettings_atom_name; XInternAtoms (dpyinfo->display, atom_names, total_atom_count, -- 2.39.2