bugGNU roff - Bugs: bug #55257, xtotroff.c: Defined size of the...

 
 

bug #55257: xtotroff.c: Defined size of the array "encoding" is too short

Submitter:  Bjarni Ingi Gislason <bjarniig>
Submitted:  Thu 20 Dec 2018 08:54:01 PM UTC
   
 
Category:  Font - others/general Severity:  3 - Normal
Item Group:  Warning/Suspicious behaviour Status:  Fixed
Privacy:  Public Assigned to:  gbranden
Open/Closed:  Closed Planned Release:  1.23.0
* Mandatory Fields

Add a New Comment Rich Markup
   

Sat 05 Dec 2020 03:29:53 PM UTC, comment #4: 

I forgot the bug-closing annotation in the ChangeLog; I'll fix that in a separate commit.


commit 29385650a04a339dbb49b684eb7b2d40ed5b6beb
Author: G. Branden Robinson <g.branden.robinson@gmail.com>
Date:   Tue Dec 1 16:37:16 2020 +1100

    [xtotroff]: Avoid overrunning buffer write.

    * src/utils/xtotroff/xtotroff.c (MapFont): Avoid writing past
      the end of a static buffer.  Problem found and patch supplied by
      Bjarni Ingi Gislason.  I tweaked it to comment it differently (in case
      the buffer ever needs to grow, but the prospects of future X11
      server-side font rendering development seem dim) and use snprintf()
      instead of retaining the existing sprintf().

      Quiets warning: '%s' directive writing up to 255 bytes into a region
      of size between 0 and 255 [-Wformat-overflow=].


G. Branden Robinson <gbranden>
Group administrator
Tue 01 Dec 2020 05:42:41 AM UTC, comment #3: 

I have a fix in preparation.  Should land with my next batch of commits.


diff --git a/ChangeLog b/ChangeLog
index b41d1198..c36954e5 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,12 @@
+2020-12-01  G. Branden Robinson <g.branden.robinson@gmail.com>
+
+       * src/utils/xtotroff/xtotroff.c (MapFont): Avoid writing past
+       the end of a static buffer.  Problem found and patch supplied by
+       Bjarni Ingi Gislason.  I tweaked it to comment it differently
+       {in case the buffer ever needs to grow, but the prospects of
+       future X11 server-side font rendering development seem dim} and
+       use snprintf() instead of retaining the existing sprinf().
+
 2020-12-01  G. Branden Robinson <g.branden.robinson@gmail.com>

        * src/utils/xtotroff/xtotroff.c (CanonicalizeFontName,
diff --git a/src/utils/xtotroff/xtotroff.c b/src/utils/xtotroff/xtotroff.c
index f6d316dc..cf7c41b8 100644
--- a/src/utils/xtotroff/xtotroff.c
+++ b/src/utils/xtotroff/xtotroff.c
@@ -130,7 +130,9 @@ static int MapFont(char *font_name, const char *troff_name)
   XFontName parsed;
   int j, k;
   DviCharNameMap *char_map;
-  char encoding[256];
+  /* 'encoding' needs to hold a CharSetRegistry (256), a CharSetEncoding
+     (256) [both from XFontName.h], a dash, and a null terminator. */
+  char encoding[256 * 2 + 1 + 1];
   char *s;
   int wid;
   char name_string[2048];
@@ -161,7 +163,8 @@ static int MapFont(char *font_name, const char *troff_name)
     return 0;

   XParseFontName(names[0], &parsed, &attributes);
-  sprintf(encoding, "%s-%s", parsed.CharSetRegistry,
+  size_t sz = sizeof encoding;
+  snprintf(encoding, sz, "%s-%s", parsed.CharSetRegistry,
          parsed.CharSetEncoding);
   for (s = encoding; *s; s++)
     if (isupper(*s))


G. Branden Robinson <gbranden>
Group administrator
Tue 01 Dec 2020 12:38:09 AM UTC, comment #2: 

  The patch that I have been using:

--- /tmp/xtotroff.c        2020-12-01 00:05:44.000000000 +0000
+++ xtotroff.c        2020-12-01 00:06:08.000000000 +0000
@@ -127,7 +127,7 @@ static int MapFont(char *font_name, cons
   XFontName parsed;
   int j, k;
   DviCharNameMap *char_map;
-  char encoding[256];
+  char encoding[2*256+1+1]; /* Format '%s-%s', each %s is 256 bytes */
   char *s;
   int wid;
   char name_string[2048];


Bjarni Ingi Gislason <bjarniig>
Mon 20 Jan 2020 12:57:11 AM UTC, comment #1: 

  I was somehow in my master branch when I issued a "make check".
I thus got a warning that I had already reported.

  I had fixed this, with a comment, in my branch.

  The fix is so simple, that keeping the issue unfixed and the report
open is a waste.


Bjarni Ingi Gislason <bjarniig>
Thu 20 Dec 2018 08:54:01 PM UTC, original submission:  

From: Bjarni Ingi Gislason <bjarniig@rhi.hi.is>
Date: Thu, 20 Dec 2018 20:47:07 +0000
Subject: xtotroff.c: Defined size of the array "encoding" is too short

  The X11 part of the groff software is only compiled if the needed
libraries are found, see README.

  Possible warnings from the compiler:

../src/utils/xtotroff/xtotroff.c: In function 'MapFont':
../src/utils/xtotroff/xtotroff.c:159:25: warning: '%s' directive writing
up to 255 bytes into a region of size between 0 and 255
[-Wformat-overflow=]
   sprintf(encoding, "%s-%s", parsed.CharSetRegistry,
                         ^~
    parsed.CharSetEncoding);
    ~~~~~~~~~~~~~~~~~~~~~~
../src/utils/xtotroff/xtotroff.c:159:3: note: 'sprintf' output between 2
and 512 bytes into a destination of size 256
   sprintf(encoding, "%s-%s", parsed.CharSetRegistry,
   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    parsed.CharSetEncoding);
    ~~~~~~~~~~~~~~~~~~~~~~~

  The size of "encoding" needs thus to be the sizeof(CharSetRegistry) +
sizeof(CharSetEncoding) + 1 (+1 for the terminating null byte?), which
sizes are defined in "src/include/XFontName.h".

Bjarni Ingi Gislason <bjarniig>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by gbranden (Updated the item)
  • -email is unavailable- added by bjarniig (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only logged-in users can vote.

     

    Follow 8 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2022-06-15 gbranden CategoryNone Font - others/general
    2020-12-05 gbranden StatusIn Progress Fixed
        Open/ClosedOpen Closed
        Planned ReleaseNone 1.23.0
    2020-12-01 gbranden StatusConfirmed In Progress
    2020-12-01 gbranden Item GroupBuild/Installation Warning/Suspicious behaviour
        StatusNone Confirmed
        Assigned toNone gbranden

    Back to the top

    Powered by Savane 3.13-f8d8.
    Corresponding source code