[groff] 22/24: [troff]: Improve input validation.

[G. Branden Robinson]

gbranden pushed a commit to branch master
in repository groff.

commit c6fe97b3d4e02fd799ededfcd59922fd28d40b16
Author: G. Branden Robinson <g.branden.robinson@gmail.com>
AuthorDate: Sun Nov 10 11:50:23 2024 -0600

    [troff]: Improve input validation.
    
    Use `has_arg()` (inverted if necessary) more often to better distinguish
    between arguments that are truly absent and those for which attempts to
    read "symbol" (identifier) names result in their objects' `is_null()`
    member functions returning true, which happens both for absent and for
    syntactically invalid identifiers.
    
    * src/roff/troff/reg.cpp (define_register_request)
      (assign_register_format_request, alias_register_request): Remove
      assertions that get tripped on invalid input.
    
      (assign_register_format_request): Skip remainder of input line and
      return early if the first argument is an invalid identifier.
      (remove_register_request): Explicitly break out of loop as soon as we
      run out of arguments.
    
      (alias_register_request, rename_register_request): Throw warning in
      category "missing" only if second argument is truly missing, not if it
      is invalid; in this scenario, `get_name()` has already thrown an error
      diagnostic.
    
      (alias_register_request): Check second argument for validity as
      identifier before attempting to look it up in the register dictionary.
    
    Continues commit 2f6a72b9e3, 3 September.
    
    Exhibit A:
    
    $ nl ATTIC/missing-args-reg.groff
         1  .af
         2  .af a
         3  .aln
         4  .aln a
         5  .nr
         6  .nr a
         7  .rnn
         8  .rnn a
         9  .rr
    $ ./build/test-groff -ww ATTIC/missing-args-reg.groff
    troff:ATTIC/missing-args-reg.groff:1: warning: register interpolation 
format assignment request expects arguments
    troff:ATTIC/missing-args-reg.groff:2: warning: register interpolation 
format assignment request register format as second argument
    troff:ATTIC/missing-args-reg.groff:3: warning: register aliasing request 
expects arguments
    troff:ATTIC/missing-args-reg.groff:4: warning: register aliasing request 
expects identifier of existing register as second argument
    troff:ATTIC/missing-args-reg.groff:5: warning: register definition request 
expects arguments
    troff:ATTIC/missing-args-reg.groff:6: warning: register definition request 
expects a numeric expression as second argument
    troff:ATTIC/missing-args-reg.groff:7: warning: register renaming request 
expects arguments
    troff:ATTIC/missing-args-reg.groff:8: warning: register renaming request 
exepects new identifier as second argument
    troff:ATTIC/missing-args-reg.groff:9: warning: register removal request 
expects arguments
    
    Exhibit B:
    
    $ nl ATTIC/bogus-args-reg.groff
         1  .af \{
         2  .af a \{
         3  .aln \{
         4  .aln a \{
         5  .nr \{
         6  .nr a \{
         7  .rnn \{
         8  .rnn a \{
         9  .rr \{
        10  .rr a \{
    $ ./build/test-groff -ww ATTIC/bogus-args-reg.groff
    troff:ATTIC/bogus-args-reg.groff:1: error: expected identifier, got an 
escaped '{'; treated as missing
    troff:ATTIC/bogus-args-reg.groff:2: error: register interpolation format 
assignment request expects 'i', 'I', 'a', 'A', or decimal digits, got an 
escaped '{'
    troff:ATTIC/bogus-args-reg.groff:3: error: expected identifier, got an 
escaped '{'; treated as missing
    troff:ATTIC/bogus-args-reg.groff:4: error: expected identifier, got an 
escaped '{'; treated as missing
    troff:ATTIC/bogus-args-reg.groff:5: error: expected identifier, got an 
escaped '{'; treated as missing
    troff:ATTIC/bogus-args-reg.groff:6: warning: expected numeric expression, 
got an escaped '{'
    troff:ATTIC/bogus-args-reg.groff:7: error: expected identifier, got an 
escaped '{'; treated as missing
    troff:ATTIC/bogus-args-reg.groff:8: error: expected identifier, got an 
escaped '{'; treated as missing
    troff:ATTIC/bogus-args-reg.groff:9: error: expected identifier, got an 
escaped '{'; treated as missing
    troff:ATTIC/bogus-args-reg.groff:10: error: expected identifier, got an 
escaped '{'; treated as missing
---
 ChangeLog              | 24 ++++++++++++++++++++++++
 src/roff/troff/reg.cpp | 37 +++++++++++++++++++++++++------------
 2 files changed, 49 insertions(+), 12 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 9ab6b3286..5ef58af19 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,27 @@
+2024-11-10  G. Branden Robinson <g.branden.robinson@gmail.com>
+
+       [troff]: Improve input validation.  Use `has_arg()` (inverted if
+       necessary) more often to better distinguish between arguments
+       that are truly absent and those for which attempts to read
+       "symbol" (identifier) names result in their objects' `is_null()`
+       member functions returning true, which happens both for absent
+       and for syntactically invalid identifiers.
+
+       * src/roff/troff/reg.cpp (define_register_request)
+       (assign_register_format_request, alias_register_request): Remove
+       assertions that get tripped on invalid input.
+       (assign_register_format_request): Skip remainder of input line
+       and return early if the first argument is an invalid identifier.
+       (remove_register_request): Explicitly break out of loop as soon
+       as we run out of arguments.
+       (alias_register_request, rename_register_request): Throw warning
+       in category "missing" only if second argument is truly missing,
+       not if it is invalid; in this scenario, `get_name()` has already
+       thrown an error diagnostic.
+       (alias_register_request): Check second argument for validity as
+       identifier before attempting to look it up in the register
+       dictionary.
+
 2024-11-09  G. Branden Robinson <g.branden.robinson@gmail.com>
 
        * src/roff/troff/env.cpp (add_hyphenation_exceptions):
diff --git a/src/roff/troff/reg.cpp b/src/roff/troff/reg.cpp
index dab110f0a..14c5ad6fc 100644
--- a/src/roff/troff/reg.cpp
+++ b/src/roff/troff/reg.cpp
@@ -314,8 +314,8 @@ void define_register_request()
     return;
   }
   symbol nm = get_name();
-  assert(nm != 0 /* nullptr */);
   if (nm.is_null()) {
+    // get_name() has already thrown an error diagnostic on bogus input.
     skip_line();
     return;
   }
@@ -418,7 +418,11 @@ void assign_register_format_request()
     return;
   }
   symbol nm = get_name();
-  assert(nm != 0 /* nullptr */);
+  if (nm.is_null()) {
+    // get_name() has already thrown an error diagnostic on bogus input.
+    skip_line();
+    return;
+  }
   reg *r = static_cast<reg *>(register_dictionary.lookup(nm));
   if (0 /* nullptr */ == r) {
     r = new number_reg;
@@ -455,9 +459,12 @@ void remove_register_request()
   }
   for (;;) {
     symbol s = get_name();
+    // get_name() has already thrown an error diagnostic on bogus input.
     if (s.is_null())
       break;
     register_dictionary.remove(s);
+    if (!has_arg())
+      break;
   }
   skip_line();
 }
@@ -471,15 +478,19 @@ void alias_register_request()
     return;
   }
   symbol s1 = get_name();
-  assert(s1 != 0 /* nullptr */);
+  // get_name() has already thrown an error diagnostic on bogus input.
   if (!s1.is_null()) {
-    symbol s2 = get_name();
-    if (s2.is_null())
+    if (!has_arg())
       warning(WARN_MISSING, "register aliasing request expects"
              " identifier of existing register as second argument");
     else {
-      if (!register_dictionary.alias(s1, s2))
-       error("cannot alias undefined register '%1'", s2.contents());
+      symbol s2 = get_name();
+      // get_name() has already thrown an error diagnostic on bogus
+      // input.
+      if (!s2.is_null()) {
+       if (!register_dictionary.alias(s1, s2))
+         error("cannot alias undefined register '%1'", s2.contents());
+      }
     }
   }
   skip_line();
@@ -494,12 +505,14 @@ void rename_register_request()
     return;
   }
   symbol s1 = get_name();
-  if (!s1.is_null()) {
+  // get_name() has already thrown an error diagnostic on bogus input.
+  if (!has_arg())
+    warning(WARN_MISSING, "register renaming request exepects new"
+           " identifier as second argument");
+  else if (!s1.is_null()) {
     symbol s2 = get_name();
-    if (s2.is_null())
-      warning(WARN_MISSING, "register renaming request exepects new"
-             " identifier as second argument");
-    else
+    // get_name() has already thrown an error diagnostic on bogus input.
+    if (!s2.is_null())
       register_dictionary.rename(s1, s2);
   }
   skip_line();