[bug #61043] potential integer overflow vulnerability in src/preproc/grn/hdb.cpp

[G. Branden Robinson]

Follow-up Comment #2, bug #61043 (project groff):

I've prepared a patch that mitigates the impact of the underlying bug.  Here
it is.

I have not pushed this yet; I am making some other changes to address the
sloppy error handing of this program.


diff --git a/ChangeLog b/ChangeLog
index 6b5c245b..7bdee9a0 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,15 @@
+2021-08-16  G. Branden Robinson <g.branden.robinson@gmail.com>
+
+       * src/preproc/grn/hdb.cpp (DBRead): Check return value of
+       `sscanf()` and call `fatal()` if no conversions succeeded.  The
+       blithe discard of a useful return value is bad enough, but this
+       one took place inside a do-while such that it could loop
+       forever trying fruitlessly to parse two doubles out of strings
+       that didn't contain them (the loop never checked the EOF status
+       of the file stream from which it was reading, and relied on
+       `fgets()` to keep advancing the stream pointer).  Discovered
+       while root-causing Savannah #61043.
+
 2021-08-15  G. Branden Robinson <g.branden.robinson@gmail.com>
 
        Resolve compiler warnings relating to format string security and
diff --git a/src/preproc/grn/hdb.cpp b/src/preproc/grn/hdb.cpp
index c61e099b..0310d7ac 100644
--- a/src/preproc/grn/hdb.cpp
+++ b/src/preproc/grn/hdb.cpp
@@ -148,7 +148,11 @@ DBRead(register FILE *file)
          if (string[0] == '*') {       /* SUN gremlin file */
            lastpoint = TRUE;
          } else {
-           (void) sscanf(string, "%lf%lf", &x, &y);
+           if (!sscanf(string, "%lf%lf", &x, &y)) {
+             error("expected coordinate pair, got '%1';"
+                   " giving up on this picture", string);
+             return(elist);
+           }
            if ((x == -1.00 && y == -1.00) && (!SUNFILE))
              lastpoint = TRUE;
            else {


Now, instead of looping forever, I get the following output.


$ ./build/test-groff -g -me -z gremlin.me
grn: error: expected coordinate pair, got 'CENTCENT'; giving up on this
picture


    _______________________________________________________

Reply to this item at:

  <https://savannah.gnu.org/bugs/?61043>

_______________________________________________
  Message sent via Savannah
  https://savannah.gnu.org/