From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Paul Eggert Newsgroups: gmane.emacs.bugs Subject: bug#9412: sprintf-related integer and memory overflow issues Date: Tue, 30 Aug 2011 23:03:07 -0700 Organization: UCLA Computer Science Department Message-ID: <4E5DCE9B.9060903@cs.ucla.edu> References: <4E5D6772.7060208@cs.ucla.edu> <87vcteib8y.fsf@stupidchicken.com> NNTP-Posting-Host: lo.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Trace: dough.gmane.org 1314770642 4115 80.91.229.12 (31 Aug 2011 06:04:02 GMT) X-Complaints-To: usenet@dough.gmane.org NNTP-Posting-Date: Wed, 31 Aug 2011 06:04:02 +0000 (UTC) Cc: 9412@debbugs.gnu.org To: Chong Yidong Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Wed Aug 31 08:03:57 2011 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane.org Original-Received: from lists.gnu.org ([140.186.70.17]) by lo.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1Qydu1-0001nH-5f for geb-bug-gnu-emacs@m.gmane.org; Wed, 31 Aug 2011 08:03:57 +0200 Original-Received: from localhost ([::1]:38869 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Qydu0-0000Ne-4E for geb-bug-gnu-emacs@m.gmane.org; Wed, 31 Aug 2011 02:03:56 -0400 Original-Received: from eggs.gnu.org ([140.186.70.92]:42338) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Qydtx-0000NY-9r for bug-gnu-emacs@gnu.org; Wed, 31 Aug 2011 02:03:54 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Qydtw-0006dg-0j for bug-gnu-emacs@gnu.org; Wed, 31 Aug 2011 02:03:53 -0400 Original-Received: from debbugs.gnu.org ([140.186.70.43]:59773) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Qydtv-0006da-Ty for bug-gnu-emacs@gnu.org; Wed, 31 Aug 2011 02:03:51 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.69) (envelope-from ) id 1Qydx0-00053L-Gk; Wed, 31 Aug 2011 02:07:02 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Paul Eggert Original-Sender: debbugs-submit-bounces@debbugs.gnu.org Resent-To: owner@debbugs.gnu.org Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Wed, 31 Aug 2011 06:07:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 9412 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch Original-Received: via spool by 9412-submit@debbugs.gnu.org id=B9412.131477080919404 (code B ref 9412); Wed, 31 Aug 2011 06:07:02 +0000 Original-Received: (at 9412) by debbugs.gnu.org; 31 Aug 2011 06:06:49 +0000 Original-Received: from localhost ([127.0.0.1] helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1Qydwm-00052u-7d for submit@debbugs.gnu.org; Wed, 31 Aug 2011 02:06:48 -0400 Original-Received: from smtp.cs.ucla.edu ([131.179.128.62]) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1QydwR-00052Q-0N for 9412@debbugs.gnu.org; Wed, 31 Aug 2011 02:06:30 -0400 Original-Received: from localhost (localhost.localdomain [127.0.0.1]) by smtp.cs.ucla.edu (Postfix) with ESMTP id CFC08A60001; Tue, 30 Aug 2011 23:03:14 -0700 (PDT) X-Virus-Scanned: amavisd-new at smtp.cs.ucla.edu Original-Received: from smtp.cs.ucla.edu ([127.0.0.1]) by localhost (smtp.cs.ucla.edu [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id QFZCw8dWpsmg; Tue, 30 Aug 2011 23:03:14 -0700 (PDT) Original-Received: from [192.168.1.10] (pool-71-189-109-235.lsanca.fios.verizon.net [71.189.109.235]) by smtp.cs.ucla.edu (Postfix) with ESMTPSA id 292BA39E80CF; Tue, 30 Aug 2011 23:03:14 -0700 (PDT) User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2.20) Gecko/20110805 Thunderbird/3.1.12 In-Reply-To: <87vcteib8y.fsf@stupidchicken.com> X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.11 Precedence: list Resent-Date: Wed, 31 Aug 2011 02:07:02 -0400 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6 (newer, 1) X-Received-From: 140.186.70.43 X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Original-Sender: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Xref: news.gmane.org gmane.emacs.bugs:50459 Archived-At: 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,