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