bugGNU roff - Bugs: bug #64205, [mm] `INITI` validates arguments...

 
 

bug #64205: [mm] `INITI` validates arguments poorly

Submitter:  G. Branden Robinson <gbranden>
Submitted:  Wed 17 May 2023 07:12:17 AM UTC
   
 
Category:  Macro mm Severity:  3 - Normal
Item Group:  Warning/Suspicious behaviour Status:  Fixed
Privacy:  Public Assigned to:  gbranden
Open/Closed:  Closed Planned Release:  1.24.0
* Mandatory Fields

Add a New Comment Rich Markup
   

Mon 10 Jul 2023 08:51:09 AM UTC, comment #3: 


commit 504d18bae32c8dc7ef7e12dcd7e4362edc80d14e
Author: G. Branden Robinson <g.branden.robinson@gmail.com>
Date:   Thu May 18 03:38:54 2023 -0500

    [mm]: Fix Savannah #64205.

    [mm]: Improve input validation of `INITI`, `IND` macros.

    * contrib/mm/m.tmac (INITI): Validate the index type (first argument)
      here instead of only later when `IND` is called.  Actually verify that
      the second argument is present and usable.

      (IND): Verify that we're given arguments.

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

    Also make some design observations in comments.

    Tested as follows using groff 1.22.4 and Git HEAD (on my branch).

    $ cat EXPERIMENTS/test-index.sh
    \#!/bin/sh

    groff=./build/test-groff
    groff=groff

    printf '.INITI\n' | $groff -mm
    printf '.INITI "" ""\n' | $groff -mm
    printf '.INITI A ""\n' | $groff -mm
    printf '.INITI N ""\n' | $groff -mm
    printf '.INITI N myindex\n.INITI N myotherindex\n' | $groff -mm
    printf '.IND foo\n' | $groff -mm
    printf '.INITI N myindex\n.IND\n' | $groff -mm
    printf '.INITI N myindex\n.IND ""\n' | $groff -mm
    printf '.INITI N myindex\n.IND foo\n' | $groff -mm
    $ sh EXPERIMENTS/test-index.sh
    ******************
    ERROR:(<standard input>) input line 1:INITI:type missing
    ******************
    "Input aborted, syntax error"
    ******************
    ERROR:(<standard input>) input line 2:INITI:file already set
    ******************
    "Input aborted, syntax error"
    ******************
    ERROR:(<standard input>) input line 1:IND: No active INITI
    ******************
    "Input aborted, syntax error"

    Removing the second assignment to the "groff" shell variable...

    $ sh EXPERIMENTS/test-index.sh
    m.tmac:<standard input>:1: error: INITI: expected 2 or 3 arguments, got 0
    m.tmac:<standard input>:1: error: INITI: invalid index type ''
    m.tmac:<standard input>:1: error: INITI: invalid index type 'A'
    m.tmac:<standard input>:1: error: INITI: index file name not specified
    m.tmac:<standard input>:2: error: INITI: index file name already set
    m.tmac:<standard input>:1: error: IND: no active index; call INITI first
    m.tmac:<standard input>:2: error: IND: expected at least 1 argument, got 0
    m.tmac:<standard input>:2: error: IND: cannot record empty index entry


G. Branden Robinson <gbranden>
Group administrator
Mon 22 May 2023 07:56:34 AM UTC, comment #2: 
G. Branden Robinson <gbranden>
Group administrator
Thu 18 May 2023 08:45:03 AM UTC, comment #1: 

Fixed in my working copy.


commit 813d91905a585673317c3beebc1380759ae91c49
Author: G. Branden Robinson <g.branden.robinson@gmail.com>
Date:   Thu May 18 03:38:54 2023 -0500

    [mm]: Fix Savannah #64205.

    [mm]: Improve input validation of `INITI`, `IND` macros.

    * m.tmac (INITI): Validate the index type (first argument) here instead
      of only later when `IND` is called.  Actually verify that the second
      argument is present and usable.

      (IND): Verify that we're given arguments.

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

    Also make some design observations in comments.

    Tested as follows using groff 1.22.4 and Git HEAD (on my branch).

    $ cat EXPERIMENTS/test-index.sh
    \#!/bin/sh

    groff=./build/test-groff
    groff=groff

    printf '.INITI\n' | $groff -mm
    printf '.INITI "" ""\n' | $groff -mm
    printf '.INITI A ""\n' | $groff -mm
    printf '.INITI N ""\n' | $groff -mm
    printf '.INITI N myindex\n.INITI N myotherindex\n' | $groff -mm
    printf '.IND foo\n' | $groff -mm
    printf '.INITI N myindex\n.IND\n' | $groff -mm
    printf '.INITI N myindex\n.IND ""\n' | $groff -mm
    printf '.INITI N myindex\n.IND foo\n' | $groff -mm
    $ sh EXPERIMENTS/test-index.sh
    ******************
    ERROR:(<standard input>) input line 1:INITI:type missing
    ******************
    "Input aborted, syntax error"
    ******************
    ERROR:(<standard input>) input line 2:INITI:file already set
    ******************
    "Input aborted, syntax error"
    ******************
    ERROR:(<standard input>) input line 1:IND: No active INITI
    ******************
    "Input aborted, syntax error"

    Removing the second assignment to the "groff" shell variable...

    $ sh EXPERIMENTS/test-index.sh
    m.tmac:<standard input>:1: error: INITI: expected 2 or 3 arguments, got 0
    m.tmac:<standard input>:1: error: INITI: invalid index type ''
    m.tmac:<standard input>:1: error: INITI: invalid index type 'A'
    m.tmac:<standard input>:1: error: INITI: index file name not specified
    m.tmac:<standard input>:2: error: INITI: index file name already set
    m.tmac:<standard input>:1: error: IND: no active index; call INITI first
    m.tmac:<standard input>:2: error: IND: expected at least 1 argument, got 0
    m.tmac:<standard input>:2: error: IND: cannot record empty index entry


G. Branden Robinson <gbranden>
Group administrator
Wed 17 May 2023 07:12:17 AM UTC, original submission:  

Affects groff 1.22.4.


.de INITI
.if \\n[.$]<2 .@error \\$0: expected 2 or 3 arguments, got \\n[.$]
.\" ignore if INITI has already been used
.if d ind*file .@error \\$0: index file name already set
.ds ind*file \\$2.ind
.if \\n[D]>1 .tm INITI: source \\*[ind*file]
.if !d ind*file .@error \\$0: index file name not specified
.ds ind*type \\$1


The "index file name not specified" error will never be thrown; `ind*file` is always defined (and with a non-empty value, though that doesn't matter for the 'd' conditional expression operator; empty strings exist nevertheless).

Also the content of the first argument is not validated at all, but blindly assigned to `ind*type`.


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

Items that depend on this one: None found

 

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 6 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2023-07-10 gbranden StatusReady for Merge Fixed
        Open/ClosedOpen Closed
        Planned ReleaseNone 1.24.0
    2023-05-22 gbranden StatusIn Progress Ready for Merge
    2023-05-18 gbranden StatusNone In Progress
        Assigned toNone gbranden

    Back to the top

    Powered by Savane 3.13-3230.
    Corresponding source code