[groff] 11/13: [tbl]: Fix potential SEGV in recently added code.

[G. Branden Robinson]

gbranden pushed a commit to branch master
in repository groff.

commit 207723e5bfad7fe77265815784364f1a4189f80e
Author: G. Branden Robinson <g.branden.robinson@gmail.com>
AuthorDate: Sun Oct 6 12:35:27 2024 -0500

    [tbl]: Fix potential SEGV in recently added code.
    
    * src/preproc/tbl/table.cpp (table::add_entry): Refactor to avoid
      overreading memory allocated to `string` objects, which don't
      null-terminate their contents.  Use string class's existing `search`
      and new `find` member functions to perform character and substring
      matches, respectively, instead of `strchr()` and `strstr()`.  As a
      side benefit, the code now looks cleaner and reads easier.
    
    Fixes a SEGV reported by Lennart Jablonka that I can't reproduce but
    which made sense once he pointed it out.  Problem introduced by me in
    commit a91cd457d9, 26 September.
---
 ChangeLog                 | 14 ++++++++++++++
 src/preproc/tbl/table.cpp | 38 ++++++++++++++++++--------------------
 2 files changed, 32 insertions(+), 20 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index efc768219..f7a91de07 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,17 @@
+2024-10-06  G. Branden Robinson <g.branden.robinson@gmail.com>
+
+       * src/preproc/tbl/table.cpp (table::add_entry): Refactor to
+       avoid overreading memory allocated to `string` objects, which
+       don't null-terminate their contents.  Use string class's
+       existing `search` and new `find` member functions to perform
+       character and substring matches, respectively, instead of
+       `strchr()` and `strstr()`.  As a side benefit, the code now
+       looks cleaner and reads easier.
+
+       Fixes a SEGV reported by Lennart Jablonka that I can't reproduce
+       but which made sense once he pointed it out.  Problem introduced
+       by me in commit a91cd457d9, 26 September.
+
 2024-10-06  G. Branden Robinson <g.branden.robinson@gmail.com>
 
        * src/include/stringclass.h (class string): Declare new `find`
diff --git a/src/preproc/tbl/table.cpp b/src/preproc/tbl/table.cpp
index 4a3b30b03..d0a19beca 100644
--- a/src/preproc/tbl/table.cpp
+++ b/src/preproc/tbl/table.cpp
@@ -1528,35 +1528,33 @@ void table::add_entry(int r, int c, const string &str,
   int len = str.length();
   // Diagnose escape sequences that can wreak havoc in generated output.
   if (len > 1) {
-    const char *entryptr = str.contents();
     // A comment on a control line or in a text block is okay.
-    const char *commentptr = strstr(entryptr, "\\\"");
-    if (commentptr != 0 /* nullptr */) {
-      const char *controlptr = strchr(entryptr, '.');
-      if ((controlptr == 0 /* nullptr */)
-         || (controlptr == entryptr)
-         || (strstr(entryptr, "\n") == 0 /* nullptr */))
+    int commentpos = str.find("\\\"");
+    if (commentpos != -1) {
+      int controlpos = str.search('.');
+      if ((-1 == controlpos)
+         || (0 == controlpos)
+         || (-1 == str.search('\n')))
        warning_with_file_and_line(fn, ln, "table entry contains"
                                   " comment escape sequence '\\\"'");
     }
-    const char *gcommentptr = strstr(entryptr, "\\#");
+    int gcommentpos = str.find("\\#");
     // If both types of comment are present, the first is what matters.
-    if ((gcommentptr != 0 /* nullptr */)
-       && (gcommentptr < commentptr))
-      commentptr = gcommentptr;
-    if (commentptr != 0 /* nullptr */) {
-      const char *controlptr = strchr(entryptr, '.');
-      if ((controlptr == 0 /* nullptr */)
-         || (controlptr == entryptr)
-         || (strstr(entryptr, "\n") == 0 /* nullptr */))
+    if ((gcommentpos != -1) && (gcommentpos < commentpos))
+      commentpos = gcommentpos;
+    if (commentpos != -1) {
+      int controlpos = str.search('.');
+      if ((-1 == controlpos)
+         || (0 == controlpos)
+         || (-1 == str.search('\n')))
        warning_with_file_and_line(fn, ln, "table entry contains"
                                   " comment escape sequence '\\#'");
     }
     // A \! escape sequence after a comment has started is okay.
-    const char *exclptr = strstr(str.contents(), "\\!");
-    if ((exclptr != 0 /* nullptr */)
-       && ((0 /* nullptr */ == commentptr)
-           || (exclptr < commentptr)))
+    int exclpos = str.find("\\!");
+    if ((exclpos != -1)
+       && ((-1 == commentpos)
+           || (exclpos < commentpos)))
       warning_with_file_and_line(fn, ln, "table entry contains"
                                 " transparent throughput escape"
                                 " sequence '\\!'");