[groff] 08/08: [troff]: Add sanity check to prevent SEGV.

[G. Branden Robinson]

gbranden pushed a commit to branch master
in repository groff.

commit 01a65469b56f7284041365b0b179b6b4b9153b9c
Author: G. Branden Robinson <g.branden.robinson@gmail.com>
AuthorDate: Thu Jul 29 19:15:40 2021 +1000

    [troff]: Add sanity check to prevent SEGV.
    
    * src/roff/troff/node.cpp (suppress_node::tprint): If emitting
      bounding box limits for an image filename, ensure that
      `image_filename` is not a null pointer (and furthermore that it's not
      a zero-length string even if the pointer is valid).  Emit an error
      diagnostic and skip further processing of the node if they are.  This
      problem implies nonsensical input (so we should guard against it), but
      I cannot reproduce the problem _except_ with pdfroff during a build of
      the groff tree, which has been reliably failing to relocate the table
      of contents in pdfmark.pdf since commit
      e876d4bfd193abb9a7d1fb6e76519349bded482a, 27 July.  If I extract the
      groff and troff commands that pdfroff is running and execute them in
      the build directory, they complete successfully, which suggests to me
      an environment problems or a race condition.
    
    This commit produces nonstandard indentation in the `else` brace block;
    I did that to keep the logic and scope of the change clear.
---
 ChangeLog               | 19 +++++++++++++++++++
 src/roff/troff/node.cpp |  5 +++++
 2 files changed, 24 insertions(+)

diff --git a/ChangeLog b/ChangeLog
index 113cbb5..6c34cd9 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,24 @@
 2021-07-29  G. Branden Robinson <g.branden.robinson@gmail.com>
 
+       [troff]: Add sanity check to prevent SEGV.
+
+       * src/roff/troff/node.cpp (suppress_node::tprint): If emitting
+       bounding box limits for an image filename, ensure that
+       `image_filename` is not a null pointer (and furthermore that
+       it's not a zero-length string even if the pointer is valid).
+       Emit an error diagnostic and skip further processing of the node
+       if they are if they are.  This problem implies nonsensical input
+       {so we should guard against it}, but I cannot reproduce the
+       problem _except_ with pdfroff during a build of the groff tree,
+       which has been reliably failing to relocate the table of
+       contents in pdfmark.pdf since commit
+       e876d4bfd193abb9a7d1fb6e76519349bded482a, 27 July.  If I extract
+       the groff and troff commands that pdfroff is running and execute
+       them in the build directory, they complete successfully, which
+       suggests to me an environment problems or a race condition.
+
+2021-07-29  G. Branden Robinson <g.branden.robinson@gmail.com>
+
        [troff]: Refactor some internal functions.
 
        * src/roff/troff/node.cpp: Rename two functions that have only
diff --git a/src/roff/troff/node.cpp b/src/roff/troff/node.cpp
index fdb52c7..c93d8bc 100644
--- a/src/roff/troff/node.cpp
+++ b/src/roff/troff/node.cpp
@@ -4090,6 +4090,10 @@ void suppress_node::tprint(troff_output_file *out)
   else {
     // Now check whether the suppress node requires us to issue limits.
     if (emit_limits) {
+      if ((0 == image_filename) || (strlen(image_filename) == 0))
+       error("cannot emit suppresion node placement information"
+             " without a (saved) filename; skipping");
+      else {
       const size_t namebuflen = 8192;
       char name[namebuflen] = { '\0' };
       // Jump through a flaming hoop to avoid a "format nonliteral"
@@ -4190,6 +4194,7 @@ void suppress_node::tprint(troff_output_file *out)
                name, hresolution, vresolution, get_string(".F"));
        fflush(stderr);
       }
+      }
     }
     else {
       if (is_on) {