gbranden pushed a commit to branch master
in repository groff.

commit c29c5d0dfea49f853ceb4f335166137d4efba585
Author: G. Branden Robinson <g.branden.robin...@gmail.com>
AuthorDate: Fri Feb 21 03:44:58 2025 -0600

    [libgroff]: Validate font desc character indices.
    
    ...more rigorously.
    
    * src/libs/libgroff/font.cpp (font::load): Do it.  Refer to an index as
      an "index", not a "code", in diagnostic messages.  Check for failure
      of strtol(3).  Reject indices that are out of `int` range.  Thus
      convinced that a narrowing conversion will be value-preserving, use
      C++ `static_cast` operator instead of C-style type cast.
    
    * doc/groff.texi.in (Font Description File Format):
    * man/groff_font.5.man (Font description file format): Rename "code"
      field to "index" and revise its description.
---
 ChangeLog                  | 16 ++++++++++++++++
 doc/groff.texi.in          | 17 +++++++++++------
 man/groff_font.5.man       | 19 ++++++++++++-------
 src/libs/libgroff/font.cpp | 29 +++++++++++++++++++++++++----
 4 files changed, 64 insertions(+), 17 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 0a66b1632..87278eaeb 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,19 @@
+2025-02-21  G. Branden Robinson <g.branden.robin...@gmail.com>
+
+       [libgroff]: Validate character indices in font description files
+       more rigorously.
+
+       * src/libs/libgroff/font.cpp (font::load): Do it.  Refer to an
+       index as an "index", not a "code", in diagnostic messages.
+       Check for failure of strtol(3).  Reject indices that are out of
+       `int` range.  Thus convinced that a narrowing conversion will be
+       value-preserving, use C++ `static_cast` operator instead of
+       C-style type cast.
+
+       * doc/groff.texi.in (Font Description File Format):
+       * man/groff_font.5.man (Font description file format): Rename
+       "code" field to "index" and revise its description.
+
 2025-02-21  G. Branden Robinson <g.branden.robin...@gmail.com>
 
        * src/roff/troff/input.cpp (get_charinfo_by_number): Rename
diff --git a/doc/groff.texi.in b/doc/groff.texi.in
index f09bcc6a0..bd470649e 100644
--- a/doc/groff.texi.in
+++ b/doc/groff.texi.in
@@ -464,7 +464,7 @@ Documentation License''.
 @title groff
 @subtitle The GNU implementation of @code{troff}
 @subtitle version @VERSION@
-@subtitle January 2025
+@subtitle February 2025
 @author Trent@tie{}A.@: Fisher
 @author Werner Lemberg
 @author G.@tie{}Branden Robinson
@@ -19660,7 +19660,7 @@ descriptions, one per line.  Each such glyph 
description comprises a set
 of fields separated by spaces or tabs and organized as follows.
 
 @quotation
-@var{name} @var{metrics} @var{type} @var{code} [@var{entity-name}]
+@var{name} @var{metrics} @var{type} @var{index} [@var{entity-name}]
 [@code{--} @var{comment}]
 @end quotation
 
@@ -19774,10 +19774,15 @@ means the glyph is both an ascender and a 
descender---this is true of
 parentheses in some fonts.
 @end table
 
-The @var{code} field gives a numeric identifier that the postprocessor
-uses to render the glyph.  The glyph can be specified to @code{troff}
-using this code by means of the @code{\N} escape sequence.  @var{code}
-can be any integer.@footnote{that is, any integer parsable by the C
+The
+@var{index}
+field is an integer that uniquely identifies a glyph within the font.
+The glyph can be specified to
+@code{troff} @c generic
+using this index by means of the
+@code{\N}
+escape sequence.
+The index can be any integer.@footnote{that is, any integer parsable by the C
 standard library's @cite{strtol@r{(3)}} function}
 
 The @var{entity-name} field defines an identifier for the glyph that the
diff --git a/man/groff_font.5.man b/man/groff_font.5.man
index 2c9f69986..de1f1a33e 100644
--- a/man/groff_font.5.man
+++ b/man/groff_font.5.man
@@ -9,7 +9,7 @@ device and font description files
 .\" Legal Terms
 .\" ====================================================================
 .\"
-.\" Copyright (C) 1989-2021 Free Software Foundation, Inc.
+.\" Copyright (C) 1989-2025 Free Software Foundation, Inc.
 .\"
 .\" This file is part of groff (GNU roff), which is a free software
 .\" project.
@@ -725,7 +725,7 @@ spaces or tabs and organized as follows.
 .
 .
 .IP
-.I name metrics type code
+.I name metrics type index
 .RI [ entity-name ]
 .RB [ \-\-
 .IR comment ]
@@ -928,22 +928,27 @@ parentheses in some fonts.
 .
 .P
 The
-.I code
-field gives a numeric identifier that the postprocessor uses to render
-the glyph.
+.I index
+field is an integer that uniquely identifies a glyph within the font.
 .
 The glyph can be specified to
 .I troff \" generic
-using this code by means of the
+using this index by means of the
 .B \[rs]N
 escape sequence.
 .
-The code can be any integer
+The index can be any integer
 (that is,
 any integer parsable by the C standard library's
 .MR strtol 3
 function).
 .
+An
+.I index
+is limited to the range of the C language
+.I int
+type on the system.
+.
 .
 .P
 The
diff --git a/src/libs/libgroff/font.cpp b/src/libs/libgroff/font.cpp
index f70c9d9c7..bd4fd1147 100644
--- a/src/libs/libgroff/font.cpp
+++ b/src/libs/libgroff/font.cpp
@@ -1,4 +1,4 @@
-/* Copyright (C) 1989-2024 Free Software Foundation, Inc.
+/* Copyright (C) 1989-2025 Free Software Foundation, Inc.
      Written by James Clark (j...@jclark.com)
 
 This file is part of groff.
@@ -22,6 +22,8 @@ along with this program.  If not, see 
<http://www.gnu.org/licenses/>. */
 
 #include <assert.h>
 #include <ctype.h>
+#include <errno.h>
+#include <limits.h> // INT_MAX, LONG_MAX
 #include <math.h>
 #include <stdlib.h>
 #include <wchar.h>
@@ -1167,15 +1169,34 @@ bool font::load(bool load_header_only)
          metric.type = type;
          p = strtok(0 /* nullptr */, WS);
          if (0 /* nullptr */ == p) {
-           t.error("missing code for '%1'", nm);
+           t.error("missing index for '%1'", nm);
            return false;
          }
          char *ptr;
-         metric.code = (int)strtol(p, &ptr, 0);
+         long index;
+         errno = 0;
+         index = strtol(p, &ptr, 0);
          if (ptr == p) {
-           t.error("invalid code '%1' for character '%2'", p, nm);
+           t.error("invalid index '%1' for character '%2'", p, nm);
            return false;
          }
+         if (INT_MAX != LONG_MAX) {
+           if ((index > INT_MAX) || (index < INT_MIN)) {
+             // This is a fib since INT_MIN's absolute value is one
+             // greater than INT_MAX's (on two's complement machines),
+             // but we can pass 3 arguments at most to the error()
+             // function.  Also, 31 bits ought to be enough for anyone.
+             t.error("index %1 for character '%2' is out of range;"
+                     " must be within +/-%3", p, nm, INT_MAX);
+             return false;
+           }
+         }
+         if (errno != 0) {
+           t.error("cannot convert index '%1' to integer for character"
+                   " '%2': %3", p, nm, strerror(errno));
+           return false;
+         }
+         metric.code = static_cast<int>(index);
          p = strtok(0 /* nullptr */, WS);
          if ((0 /* nullptr */ == p) || (strcmp(p, "--") == 0)) {
            metric.special_device_coding = 0;

_______________________________________________
groff-commit mailing list
groff-commit@gnu.org
https://lists.gnu.org/mailman/listinfo/groff-commit

Reply via email to