Hello Stuart,
Stuart Henderson wrote on Wed, Sep 30, 2026 at 02:05:49PM +0100:
> On 2026/09/30 14:54, Ingo Schwarze wrote:
>> i believe it might be useful to also commit the exact same textproc/groff
>> security patch that i committed to -current ports yesterday to 7.8
>> and 7.9 -stable ports as well.
>> I never committed to -stable ports before and i don't really know
>> how to test before commit because i don't have any old -stable
>> machines around. Any recommendations what i should do?
> Only the most recent -stable branch is active for ports, i.e. OPENBSD_7_9.
> After applying to that tree locally, it builds and "make test" is happy.
Great, thank you for testing!
> Either I can commit (the diff that I have is below,
I think it is the safer option that you commit what you already have
in your tree and what you have already tested somewhat (with make test)
than me applying it and committing it half-blindly.
> it matches your -current commit) or you can (cvs co -r OPENBSD_7_9
> ports/textproc/groff, cd, apply, cvs add, commit).
I checked by applying what you sent (see below) to a copy of
the textproc/groff directory with -r OPENBSD_7_9, and as expected,
the content of the two textproc/groff directories is identical
except for the CVS/Entries and CVS/Tag files. That is expected
because the groff port did not change since the 7.9 release.
So, OK schwarze@.
Thank you very much!
Ingo
> Index: Makefile
> ===================================================================
> RCS file: /cvs/ports/textproc/groff/Makefile,v
> diff -u -p -r1.52 Makefile
> --- Makefile 27 Sep 2023 18:09:00 -0000 1.52
> +++ Makefile 30 Sep 2026 12:58:42 -0000
> @@ -2,7 +2,7 @@ COMMENT = GNU troff typesetter
> VERSION = 1.22.4
> SUBST_VARS = VERSION
> DISTNAME = groff-${VERSION}
> -REVISION = 4
> +REVISION = 5
>
> CATEGORIES = textproc
> DPB_PROPERTIES = parallel
> Index: patches/patch-contrib_mm_mmroff_pl
> ===================================================================
> RCS file: patches/patch-contrib_mm_mmroff_pl
> diff -N patches/patch-contrib_mm_mmroff_pl
> --- /dev/null 1 Jan 1970 00:00:00 -0000
> +++ patches/patch-contrib_mm_mmroff_pl 30 Sep 2026 12:58:42 -0000
> @@ -0,0 +1,24 @@
> +Prevent a shell-command-injection vulnerability in mmroff(1)
> +when a malicious input file inserts a specially crafted comment line into
> +the standard output or standard error output of the first groff(1) pass.
> +
> +This security vulnerability existed since mmroff.pl was first
> +checked in on February 6, 2000, first released with groff-1.16.
> +Problem reported by Pavol Sloboda (Redhat). The fix is joint work
> +by G. Branden Robinson (GNU roff maintainer) and schwarze@.
> +Fixes <https://savannah.gnu.org/bugs/?68689>.
> +Backported from groff-1.24.2 to 1.22.4,
> +with one quoting error corrected by schwarze@.
> +
> +Index: contrib/mm/mmroff.pl
> +--- contrib/mm/mmroff.pl.orig
> ++++ contrib/mm/mmroff.pl
> +@@ -76,7 +76,7 @@ while(<MACRO>) {
> + next;
> + }
> + if (m#^\.\\" PIC (\w+)\s+(\S+)#) {
> +- eval "\$cur{'$1'} = '$2'";
> ++ $cur{"$1"} = $2;
> + next;
> + }
> + s#\\ \\ $##;
> Index: patches/patch-src_devices_gropdf_pdfmom_pl
> ===================================================================
> RCS file: patches/patch-src_devices_gropdf_pdfmom_pl
> diff -N patches/patch-src_devices_gropdf_pdfmom_pl
> --- /dev/null 1 Jan 1970 00:00:00 -0000
> +++ patches/patch-src_devices_gropdf_pdfmom_pl 30 Sep 2026 12:58:42
> -0000
> @@ -0,0 +1,82 @@
> +Fix command injection vulnerability in pdfmom(1)
> +by sanitizing command-line arguments.
> +
> +This security vulnerability existed since the pdfmom command script
> +was first checked in on August 31, 2012, first released with groff-1.22.1.
> +Problem reported by Pavol Sloboda (Redhat).
> +The fix is joint work by G. Branden Robinson (GNU roff maintainer)
> +and Deri James (groff PDF maintainer).
> +Fixes <https://savannah.gnu.org/bugs/?68687>.
> +Backported from groff-1.24.2 to 1.22.4 by schwarze@.
> +
> +Index: src/devices/gropdf/pdfmom.pl
> +--- src/devices/gropdf/pdfmom.pl.orig
> ++++ src/devices/gropdf/pdfmom.pl
> +@@ -33,9 +33,20 @@ my $RT_SEP='@RT_SEP@';
> + $ENV{PATH}=$ENV{GROFF_BIN_PATH}.$RT_SEP.$ENV{PATH} if
> exists($ENV{GROFF_BIN_PATH});
> + $ENV{TMPDIR}=$ENV{GROFF_TMPDIR} if exists($ENV{GROFF_TMPDIR});
> +
> +-while (my $c=shift)
> ++sub Clean
> + {
> +- $c=~s/(?<!\\)"/\\"/g;
> ++ my $c=shift;
> ++
> ++ if (defined($c))
> ++ {
> ++ $c=~s/'/'\\''/g;
> ++ }
> ++
> ++ return $c;
> ++}
> ++
> ++while (my $c=Clean(shift))
> ++{
> +
> + if (substr($c,0,2) eq '-T')
> + {
> +@@ -45,7 +56,7 @@ while (my $c=shift)
> + }
> + else
> + {
> +- $dev=shift;
> ++ $dev=Clean(shift);
> + }
> + next;
> + }
> +@@ -53,12 +64,12 @@ while (my $c=shift)
> + {
> + if (length($c) > 2)
> + {
> +- $preconv=$c;
> ++ $preconv.=" '$c'";
> + }
> + else
> + {
> +- $preconv=$c;
> +- $preconv.=shift;
> ++ $preconv.=" '$c";
> ++ $preconv.=Clean(shift)."'";
> + }
> + next;
> + }
> +@@ -81,8 +92,8 @@ while (my $c=shift)
> + {
> + if (length($c) > 1)
> + {
> +- push(@cmd,"\"$c\"");
> +- push(@cmd,"'".(shift)."'") if length($c)==2 and
> index('dDfFIKLmMnoPrwW',substr($c,-1)) >= 0;
> ++ push(@cmd,"'$c'");
> ++ push(@cmd,"'".Clean(shift)."'") if length($c)==2 and
> index('dDfFIKLmMnoPrwW',substr($c,-1)) >= 0;
> + }
> + else
> + {
> +@@ -96,7 +107,7 @@ while (my $c=shift)
> + {
> + # Got a filename?
> +
> +- push(@cmd,"\"$c\"");
> ++ push(@cmd,"'$c'");
> + $readstdin=0 if $readstdin == 1;
> +
> + }
> Index: patches/patch-src_preproc_html_pre-html_cpp
> ===================================================================
> RCS file: patches/patch-src_preproc_html_pre-html_cpp
> diff -N patches/patch-src_preproc_html_pre-html_cpp
> --- /dev/null 1 Jan 1970 00:00:00 -0000
> +++ patches/patch-src_preproc_html_pre-html_cpp 30 Sep 2026 12:58:42
> -0000
> @@ -0,0 +1,39 @@
> +Fix command injection vulnerability in grohtml(1) by rejecting
> +shell-significant syntax characters in raster image file names.
> +
> +This security vulnerability existed since the pre-grohtml preprocessor
> +was first checked in on January 7, 2001, first released with groff-1.17.
> +Problem reported by Pavol Sloboda (Redhat).
> +Fix by G. Branden Robinson (GNU roff maintainer).
> +Together with the troff/input.cpp patch,
> +this fixes <https://savannah.gnu.org/bugs/?68688>.
> +Backported from the Savannah ticket to 1.22.4 by schwarze@
> +with one correction by schwarze@: added exit(3) in the new NULL branch.
> +
> +Index: src/preproc/html/pre-html.cpp
> +--- src/preproc/html/pre-html.cpp.orig
> ++++ src/preproc/html/pre-html.cpp
> +@@ -1020,6 +1020,23 @@ void imageList::createImage(imageItem *i)
> + + max(i->Y1, i->Y2) * image_res / postscriptRes
> + + 1 + IMAGE_BOARDER_PIXELS;
> + if (createPage(i->pageNo) == 0) {
> ++ if (0 /* nullptr */ == i->imageName) {
> ++ fprintf(stderr, "%s: error: unable to generate raster image"
> ++ " of page %d\n", program_name, i->pageNo);
> ++ fflush(stderr);
> ++ exit(EXIT_FAILURE);
> ++ }
> ++ for (const char *p = i->imageName; *p != '\0'; p++) {
> ++ if (strchr(";&|*?~<>^()[]{}'$`\"\\#", *p)) {
> ++ char q = '\'';
> ++ if ('\'' == *p)
> ++ q = '\"';
> ++ fprintf(stderr, "%s: fatal error: unsafe shell character"
> ++ " %c%c%c in raster image file name", q, *p, q);
> ++ fflush(stderr);
> ++ exit(EXIT_FAILURE);
> ++ }
> ++ }
> + s = make_message("pnmcut%s %d %d %d %d < %s "
> + "| pnmcrop -quiet | pnmtopng%s %s > %s\n",
> + EXE_EXT,
> Index: patches/patch-src_roff_troff_input_cpp
> ===================================================================
> RCS file: patches/patch-src_roff_troff_input_cpp
> diff -N patches/patch-src_roff_troff_input_cpp
> --- /dev/null 1 Jan 1970 00:00:00 -0000
> +++ patches/patch-src_roff_troff_input_cpp 30 Sep 2026 12:58:42 -0000
> @@ -0,0 +1,35 @@
> +Fix command injection vulnerability in grohtml(1) by rejecting
> +shell-significant syntax characters in raster image file names.
> +
> +This security vulnerability existed since the pre-grohtml preprocessor
> +was first checked in on January 7, 2001, first released with groff-1.17.
> +Problem reported by Pavol Sloboda (Redhat).
> +Fix by G. Branden Robinson (GNU roff maintainer).
> +Together with the preproc/html/pre-html.cpp patch,
> +this fixes <https://savannah.gnu.org/bugs/?68688>.
> +Backported from groff-1.24.2 to 1.22.4 by schwarze@.
> +
> +Index: src/roff/troff/input.cpp
> +--- src/roff/troff/input.cpp.orig
> ++++ src/roff/troff/input.cpp
> +@@ -5507,6 +5507,20 @@ static node *do_suppress(symbol nm)
> + error("missing image name for \\O");
> + return 0;
> + }
> ++ // pre-grohtml runs troff (using the "ps" output device). It also
> ++ // has a function called `html_system()` that calls the dreaded
> ++ // system(3).
> ++ for (const char *p = s; *p != '\0'; p++) {
> ++ if (strchr(";&|*?~<>^()[]{}'$`\"\\#", *p) != 0 /* nullptr */) {
> ++ char q = '\'';
> ++ if ('\'' == *p)
> ++ q = '\"';
> ++ error("rejecting image file name argument"
> ++ " containing unsafe shell character %1%2%3"
> ++ " in output suppression escape sequence", q, *p, q);
> ++ return 0 /* nullptr */;
> ++ }
> ++ }
> + image_no++;
> + if (begin_level == 0)
> + return new suppress_node(symbol(s), position, image_no);
