bugGNU roff - Bugs: bug #65322, [troff] chopping box diversions...

 
 

bug #65322: [troff] chopping box diversions can crash the formatter

Submitter:  G. Branden Robinson <gbranden>
Submitted:  Fri 16 Feb 2024 08:57:10 PM UTC
   
 
Category:  Core Severity:  4 - Important
Item Group:  Crash/Unresponsive Status:  Unreproducible
Privacy:  Public Assigned to:  None
Open/Closed:  Open Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Sat 22 Jun 2024 03:59:15 AM UTC, comment #5: 

I found out today that the reproducer no longer works to provoke this bug.

Deri changed gropdf to stop provoking the bug as one of the "various problems" resolved by a commit of his in April.

This ticket has nothing to do with bug #65585 because this one long predates commit cd9fde325f (4 March 2024).  This bug reproduces all the way back to groff 1.22.3, well before my influence brought the codebase to ruin.  ;-)  As noted in comment #0, the crash was due to an assertion failure put in by James Clark decades ago.

Still, Deri's commit didn't change any C++ code.  So presumably it remains possible to crash the formatter with appropriately crafted input.  Both the code that Deri removed in this commit and the code at issue below (in comment #0, "pdfmomclean") involve doing surgery on diversions (box diversions only?) with the `chop` request.

So we need a new reproducer.


2387949ffc11b753dd91480dcb85be6c22819790 is the first fixed commit
commit 2387949ffc11b753dd91480dcb85be6c22819790
Author: Deri James <deri@chuzzlewit.myzen.co.uk>
Date:   Fri Apr 12 17:10:49 2024 +0100

    Correct linear search introduced in commit cd9fde325f

    * tmac/pdf.tmac: Various problems have been rectified, speed
    improvements to the linear search, Keith protection against
    illegal characters being used in string identifiers.

    * contrib/mom/om.tmac: Switch to using Branden's linear search
    so that we don't have to maintain two different methods.

    Fixes <https://savannah.gnu.org/bugs/?65585>

 ChangeLog           | 31 ++++++++++++++++++++++
 contrib/mom/om.tmac |  5 ++--
 tmac/pdf.tmac       | 76 +++++++++++++++++++++++------------------------------
 3 files changed, 67 insertions(+), 45 deletions(-)


G. Branden Robinson <gbranden>
Group administrator
Sat 17 Feb 2024 06:36:01 PM UTC, comment #4: 

I find it significant that "protecting" a leading ellipsis dot with a dummy character does nothing to keep the new reproducer from coring on any version of groff that I'm testing (see earlier comments).


$ diff -u EXPERIMENTS/heading.mom{.crashy,}
--- EXPERIMENTS/heading.mom.crashy      2024-02-17 12:27:57.427826358 -0600
+++ EXPERIMENTS/heading.mom     2024-02-17 12:28:35.767637242 -0600
@@ -3,7 +3,7 @@
 .START
 .HEADING 1 NAMED one "Attack\|.\|.\|."
 This is a heading.
-.HEADING 2 NAMED two ".\|.\|.\|Sustain\|.\|.\|."
+.HEADING 2 NAMED two "\&.\|.\|.\|Sustain\|.\|.\|."
 Let's refer back to the first heading (the one titled
 .PDF_LINK one SUFFIX ). +
 .HEADING 2 NAMED three "\&.\|.\|.\|Decay\|.\|.\|."


Find out if something deeper within mom is prepending a dummy character anyway.

If so: it seems more likely that dummy character semantics are not being handled correctly when diverted, or when a diversion is "reread" (tellingly, all of these crashes only happen after the document input has been read, and the formatter is cleaning up).

If not: the dummy character may be a red herring and something more challenging/interesting may be going wrong.

The aspect of this that has the formatter panicking because the input level isn't what it expected is, to me, strongly reminiscent of a bug elsewhere from ages ago (though not all that old when James Clark wrote groff).  Maybe this is our own SCKMUD bug.

https://www.trs-80.com/wordpress/tips/easter-eggs/

G. Branden Robinson <gbranden>
Group administrator
Sat 17 Feb 2024 06:24:19 PM UTC, comment #3: 

I should probably have clarified that my new reproducer makes groff 1.22.4 and 1.23.0 core *without patching groff in any way*.

G. Branden Robinson <gbranden>
Group administrator
Sat 17 Feb 2024 06:22:43 PM UTC, comment #2: 

Aha.  It's worse.

I can make groff 1.23.0 and 1.22.4 core as well.

This is good and bad.

It's bad that the bug exists and has been around a long time, of course.

But it's good that groff Git hasn't regressed this.  That gives us more room to throw our elbows with the solution, I think.

Reproducer:


$ cat EXPERIMENTS/heading.mom
.TITLE "My Document"
.PRINTSTYLE TYPESET
.START
.HEADING 1 NAMED one "Attack\|.\|.\|."
This is a heading.
.HEADING 2 NAMED two ".\|.\|.\|Sustain\|.\|.\|."
Let's refer back to the first heading (the one titled
.PDF_LINK one SUFFIX ). +
.HEADING 2 NAMED three "\&.\|.\|.\|Decay\|.\|.\|."
We just keep rambling on.
.HEADING 2 NAMED four "Release"
We conclude the discussion.
$ ~/groff-stable/bin/groff -b -mom -Tpdf -Z EXPERIMENTS/heading.mom | grep 'x X ps:.*exec'
x X ps: exec 0 setlinejoin
x X ps: exec 0 setlinecap
x X ps:exec [/PageMode /UseOutlines /DOCVIEW pdfmark
x X ps: exec 0 setlinejoin
x X ps: exec 0 setlinecap
x X ps:exec [/Dest /pdf:bm1 /View [/FitH -1842 u] /DEST pdfmark
x X ps:exec [/Dest /pdf:bm1 /Title (My Document) /Level 1 /OUT pdfmark
x X ps:exec [/Dest /one /View [/FitH -122316 u] /DEST pdfmark
x X ps:exec [/Dest /one /Title (Attack...) /Level 2 /OUT pdfmark
troff: backtrace: '/home/branden/groff-stable/share/groff/1.23.0/tmac/om.tmac':23454: string 'PDF_TX'
troff: backtrace: '/home/branden/groff-stable/share/groff/1.23.0/tmac/om.tmac':24024: macro 'pdfmomclean'
troff: backtrace: '/home/branden/groff-stable/share/groff/1.23.0/tmac/om.tmac':23455: macro 'PDF_BOOKMARK'
troff: backtrace: '/home/branden/groff-stable/share/groff/1.23.0/tmac/om.tmac':11082: macro 'HEADING'
troff: backtrace: file 'EXPERIMENTS/heading.mom':6
troff:EXPERIMENTS/heading.mom:6: error: expected identifier, got a horizontal motion; treated as missing
x X ps: exec decornone
troff: backtrace: '/home/branden/groff-stable/share/groff/1.23.0/tmac/pdf.tmac':256: macro 'pdfbookmark'
troff: backtrace: '/home/branden/groff-stable/share/groff/1.23.0/tmac/om.tmac':23464: macro 'PDF_BOOKMARK'
troff: backtrace: '/home/branden/groff-stable/share/groff/1.23.0/tmac/om.tmac':11082: macro 'HEADING'
troff: backtrace: file 'EXPERIMENTS/heading.mom':6
troff:EXPERIMENTS/heading.mom:6: error: expected identifier, got a horizontal motion; treated as missing
x X ps:exec [/Dest /two /View [/FitH -170790 u] /DEST pdfmark
x X ps:exec [/Dest /two /Title () /Level 3 /OUT pdfmark
x X ps: exec decornone
x X ps:exec [/Dest /three /View [/FitH -219264 u] /DEST pdfmark
x X ps:exec [/Dest /three /Title (...Decay...) /Level 3 /OUT pdfmark
x X ps: exec decornone
x X ps:exec [/Dest /four /View [/FitH -267738 u] /DEST pdfmark
x X ps:exec [/Dest /four /Title (Release) /Level 3 /OUT pdfmark
troff: ../src/roff/troff/input.cpp:523: static int input_stack::finish_get(node**): Assertion `level == 0' failed.
/home/branden/groff-stable/bin/groff: error: troff: Aborted (core dumped)
$ /usr/bin/groff -mom -Tpdf -Z EXPERIMENTS/heading.mom | grep 'x X ps:.*exec'
x X ps: exec 0 setlinejoin
x X ps: exec 0 setlinecap
x X ps:exec [/PageMode /UseOutlines /DOCVIEW pdfmark
x X ps: exec 0 setlinejoin
x X ps: exec 0 setlinecap
troff: EXPERIMENTS/heading.mom:3: can't transparently output node at top level
x X ps:exec [/Dest /pdf:bm1 /View [/FitH -1842 u] /DEST pdfmark
x X ps:exec [/Dest /pdf:bm1 /Title (My Document) /Level 1 /OUT pdfmark
troff: EXPERIMENTS/heading.mom:4: can't transparently output node at top level
troff: EXPERIMENTS/heading.mom:4: can't transparently output node at top level
troff: EXPERIMENTS/heading.mom:4: can't transparently output node at top level
x X ps:exec [/Dest /one /View [/FitH -122316 u] /DEST pdfmark
x X ps:exec [/Dest /one /Title (Attack...) /Level 2 /OUT pdfmark
troff: EXPERIMENTS/heading.mom:6: name expected (got a horizontal space): treated as missing
troff: EXPERIMENTS/heading.mom:6: name expected (got a horizontal space): treated as missing
x X ps: exec decornone
x X ps:exec [/Dest /two /View [/FitH -170790 u] /DEST pdfmark
x X ps:exec [/Dest /two /Title () /Level 3 /OUT pdfmark
x X ps: exec decornone
troff: EXPERIMENTS/heading.mom:9: can't transparently output node at top level
troff: EXPERIMENTS/heading.mom:9: can't transparently output node at top level
troff: EXPERIMENTS/heading.mom:9: can't transparently output node at top level
troff: EXPERIMENTS/heading.mom:9: can't transparently output node at top level
troff: EXPERIMENTS/heading.mom:9: can't transparently output node at top level
troff: EXPERIMENTS/heading.mom:9: can't transparently output node at top level
troff: EXPERIMENTS/heading.mom:9: can't transparently output node at top level
x X ps:exec [/Dest /three /View [/FitH -219264 u] /DEST pdfmark
x X ps:exec [/Dest /three /Title (...Decay...) /Level 3 /OUT pdfmark
x X ps: exec decornone
x X ps:exec [/Dest /four /View [/FitH -267738 u] /DEST pdfmark
x X ps:exec [/Dest /four /Title (Release) /Level 3 /OUT pdfmark
troff: Failed assertion at line 524, file '../../src/roff/troff/input.cpp'.
/usr/bin/groff: troff: Signal 6 (core dumped)


G. Branden Robinson <gbranden>
Group administrator
Fri 16 Feb 2024 10:43:06 PM UTC, comment #1: 

The bug is latent in groff 1.22.4 and 1.23.0.

To observe it, apply the patch in bug #65320.


$ cat ATTIC/pdffix.diff
diff --git a/tmac/pdf.tmac b/tmac/pdf.tmac
index 54175434f..fe12d35da 100644
--- a/tmac/pdf.tmac
+++ b/tmac/pdf.tmac
@@ -253,7 +253,7 @@ .de pdfbookmark
 .         tr \[em]-
 .         nf
 .         box pdf:clean
-.         nop \\$*
+.         nop \&\\$*
 .         fl
 .         box
 .         chop pdf:clean
$ patch -p1 < ATTIC/pdffix.diff
patching file tmac/pdf.tmac
$ ~/groff-stable/bin/groff --version
GNU groff version 1.23.0
$ ~/groff-stable/bin/groff -M ./tmac -z -mom -Tpdf ./EXPERIMENTS/heading.mom
troff: ../src/roff/troff/input.cpp:523: static int input_stack::finish_get(node**): Assertion `level == 0' failed.
/home/branden/groff-stable/bin/groff: error: troff: Aborted (core dumped)
$ /usr/bin/groff --version
GNU groff version 1.22.4
[...]
$ /usr/bin/groff -M ./tmac -z -mom -Tpdf ./EXPERIMENTS/heading.mom
troff: ./EXPERIMENTS/heading.mom:2: can't transparently output node at top level
troff: ./EXPERIMENTS/heading.mom:2: can't transparently output node at top level
troff: ./EXPERIMENTS/heading.mom:3: can't transparently output node at top level
troff: ./EXPERIMENTS/heading.mom:5: can't transparently output node at top level
troff: Failed assertion at line 524, file '../../src/roff/troff/input.cpp'.
/usr/bin/groff: troff: Signal 6 (core dumped)


G. Branden Robinson <gbranden>
Group administrator
Fri 16 Feb 2024 08:57:10 PM UTC, original submission:  

This is a problem in Git only; I don't see it in 1.23.0.

When you do certain intensely clever things with diversions, as the mom package and pdf.tmac macro file do, you can apparently confuse the formatter into the wilderness.


$ cat ./EXPERIMENTS/heading.mom
.PRINTSTYLE TYPESET
.START
.HEADING 1 NAMED one "First Heading"
This is a heading.
.HEADING 2 NAMED two "Second Heading"
Let's refer back to the first heading (this
.PDF_LINK one SUFFIX ). +
$ ./build/test-groff -mom -Tpdf ./EXPERIMENTS/heading.mom >| heading.pdf
troff: ../src/roff/troff/input.cpp:626: static int input_stack::finish_get(node**): Assertion `level == 0' failed.
/home/branden/src/GIT/groff/build/groff: error: troff: Aborted (core dumped)


That assertion was put in by James Clark in groff 1.02 (1991) or earlier.  I'm going to guess it's not spurious.

My hypothesis is that the `chop` and `asciify` requests in mom's `pdfmomclean` macro can damage a diversion's contents such that the formatter gets into an invalid state.


.de pdfmomclean
.   ie '\\n[.z]'' \{\
.      ds pdfcleaned \\$*
.      ev pdfcln
.      tr \[em]-
.      nf
.      box pdf:clean
.      nop \\*[\\*[pdfcleaned]]
.      fl
.      box
.      chop pdf:clean
.      asciify pdf:clean
.      ev
.      ds \\*[pdfcleaned] "\\*[pdf:clean]
.      rm pdf:clean
.      tr \[em]\[em]
.    \}
.    el .nop \!.pdfmomclean \\$@
..


But I place no blame on `pdfmomclean`; the formatter should not permit itself to be confused in this way.

Figuring out the details will take some digging.

G. Branden Robinson <gbranden>
Group administrator

 

(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

Digest:
   bug dependencies.

 

Carbon-Copy List
  • -email is unavailable- added by gbranden (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 5 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2024-06-22 gbranden StatusConfirmed Unreproducible
        Assigned togbranden None
        Summary[troff] certain diversion stunts can crash the formatter [troff] chopping box diversions can crash the formatter
    2024-02-17 gbranden Severity3 - Normal 4 - Important
    2024-02-17 gbranden Dependencies- bugs #65320 is dependent

    Back to the top

    Powered by Savane 3.14-3b9d.
    Corresponding source code