[bug #64301] [troff] susceptible to integer overflow

[G. Branden Robinson]

Follow-up Comment #3, bug #64301 (group groff):

Checkpoint.


commit 21e441cc615f294dec9e3f296b7e6a8f8f5bc812
Author: G. Branden Robinson <g.branden.robinson@gmail.com>
Date:   Mon Jul 15 09:44:55 2024 -0500

    XXX stdckdint bootstrap.conf (1/x)

diff --git a/bootstrap.conf b/bootstrap.conf
index 20bee83f1..0e34b056c 100644
--- a/bootstrap.conf
+++ b/bootstrap.conf
@@ -22,7 +22,7 @@ m4_base=gnulib_m4
 # gnulib C source files
 source_base=lib
 
-# additional standard files, particularly added by 
+# additional standard files, particularly added by
 # automake --add-missing
 build_aux=build-aux
 
@@ -45,6 +45,7 @@ gnulib_modules="
     vsnprintf
     stat
     stdbool-c99
+    stdckdint
     stdint
     sys_wait
 "
@@ -107,3 +108,12 @@ bootstrap_post_import_hook ()
   # Automake requires that ChangeLog exist.
   touch ChangeLog || return 1
 }
+
+##### Editor settings
+# Local Variables:
+# coding: latin-1
+# fill-column: 72
+# mode: text
+# version-control: never
+# End:
+# vim: set autoindent shiftwidth=2 textwidth=72:

commit 5ec5ecb56ca468ce9d1fd62ad00e376e3f2db5b7
Author: G. Branden Robinson <g.branden.robinson@gmail.com>
Date:   Mon Jul 15 09:45:24 2024 -0500

    XXX stdckdint number.cpp get_number, is_valid_expression (2/x)

diff --git a/src/roff/troff/number.cpp b/src/roff/troff/number.cpp
index 1c83c5316..66bb62cd2 100644
--- a/src/roff/troff/number.cpp
+++ b/src/roff/troff/number.cpp
@@ -16,6 +16,11 @@ for more details.
 You should have received a copy of the GNU General Public License
 along with this program.  If not, see <http://www.gnu.org/licenses/>. */
 
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#include <stdckdint.h>
 
 #include "troff.h"
 #include "hvunits.h"
@@ -162,10 +167,12 @@ bool get_number(units *res, unsigned char si, units
prev_value)
     *res = u;
     break;
   case INCREMENT:
-    *res = prev_value + u;
+    if (ckd_add(res, prev_value, u))
+      warning(WARN_RANGE, "integer addition saturated");
     break;
   case DECREMENT:
-    *res = prev_value - u;
+    if (ckd_sub(res, prev_value, u))
+      warning(WARN_RANGE, "integer subtraction saturated");
     break;
   default:
     assert(0 == "unhandled case returned by get_incr_number()");
@@ -183,10 +190,12 @@ bool get_integer(int *res, int prev_value)
     *res = i;
     break;
   case INCREMENT:
-    *res = prev_value + int(i);
+    if (ckd_add(res, prev_value, i))
+      warning(WARN_RANGE, "integer addition saturated");
     break;
   case DECREMENT:
-    *res = prev_value - int(i);
+    if (ckd_sub(res, prev_value, i))
+      warning(WARN_RANGE, "integer subtraction saturated");
     break;
   default:
     assert(0 == "unhandled case returned by get_incr_number()");
@@ -296,7 +305,6 @@ static bool is_valid_expression(units *u, int
scaling_unit,
     if (!is_valid_term(&u2, scaling_unit, is_parenthesized,
                       is_mandatory))
       return false;
-    bool had_overflow = false;
     switch (op) {
     case '<':
       *u = *u < u2;
@@ -328,57 +336,22 @@ static bool is_valid_expression(units *u, int
scaling_unit,
       *u = *u > 0 || u2 > 0;
       break;
     case '+':
-      if (u2 < 0) {
-       if (*u < INT_MIN - u2)
-         had_overflow = true;
-      }
-      else if (u2 > 0) {
-       if (*u > INT_MAX - u2)
-         had_overflow = true;
-      }
-      if (had_overflow) {
+      if (ckd_add(u, *u, u2)) {
        error("addition overflow");
        return false;
       }
-      *u += u2;
       break;
     case '-':
-      if (u2 < 0) {
-       if (*u > INT_MAX + u2)
-         had_overflow = true;
-      }
-      else if (u2 > 0) {
-       if (*u < INT_MIN + u2)
-         had_overflow = true;
-      }
-      if (had_overflow) {
+      if (ckd_sub(u, *u, u2)) {
        error("subtraction overflow");
        return false;
       }
-      *u -= u2;
       break;
     case '*':
-      if (u2 < 0) {
-       if (*u > 0) {
-         if ((unsigned)*u > -(unsigned)INT_MIN / -(unsigned)u2)
-           had_overflow = true;
-       }
-       else if (-(unsigned)*u > INT_MAX / -(unsigned)u2)
-         had_overflow = true;
-      }
-      else if (u2 > 0) {
-       if (*u > 0) {
-         if (*u > INT_MAX / u2)
-           had_overflow = true;
-       }
-       else if (-(unsigned)*u > -(unsigned)INT_MIN / u2)
-         had_overflow = true;
-      }
-      if (had_overflow) {
+      if (ckd_mul(u, *u, u2)) {
        error("multiplication overflow");
        return false;
       }
-      *u *= u2;
       break;
     case '/':
       if (u2 == 0) {

commit 416e7c891028ecdf5ccbe9e1cff64eae43013717
Author: G. Branden Robinson <g.branden.robinson@gmail.com>
Date:   Mon Jul 15 10:28:38 2024 -0500

    XXX stdckdint hvunits.h vunits operator - (3/x)

diff --git a/src/roff/troff/hvunits.h b/src/roff/troff/hvunits.h
index c685788df..7e20a0c86 100644
--- a/src/roff/troff/hvunits.h
+++ b/src/roff/troff/hvunits.h
@@ -16,6 +16,12 @@ for more details.
 You should have received a copy of the GNU General Public License
 along with this program.  If not, see <http://www.gnu.org/licenses/>. */
 
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#include <stdckdint.h>
+
 class vunits {
   int n;
 public:
@@ -105,14 +111,17 @@ inline vunits operator -(const vunits & x, const vunits
& y)
 {
   vunits r;
   r = x;
-  r.n -= y.n;
+  if (ckd_sub(&r.n, r.n, y.n))
+    warning(WARN_RANGE, "integer subtraction saturated");
   return r;
 }
 
 inline vunits operator -(const vunits & x)
 {
   vunits r;
-  r.n = -x.n;
+  // Why?  Consider -(INT_MIN) in two's complement.
+  if (ckd_mul(&r.n, x.n, -1))
+    warning(WARN_RANGE, "integer multiplication saturated");
   return r;
 }
 

commit f917b38491d5efce19e7449bf6a8410ffc513f53
Author: G. Branden Robinson <g.branden.robinson@gmail.com>
Date:   Mon Jul 15 09:49:57 2024 -0500

    XXX stdckdint hvunits.h vunits::to_units (4/x)

diff --git a/src/roff/troff/hvunits.h b/src/roff/troff/hvunits.h
index 7e20a0c86..1e3f860e2 100644
--- a/src/roff/troff/hvunits.h
+++ b/src/roff/troff/hvunits.h
@@ -91,7 +91,10 @@ inline vunits:: vunits() : n(0)
 
 inline units vunits::to_units()
 {
-  return n * vresolution;
+  units r;
+  if (ckd_mul(&r, n, vresolution))
+    warning(WARN_RANGE, "integer multiplication saturated");
+  return r;
 }
 
 inline bool vunits::is_zero()

commit cb45ee9be57fba38b391fd93245146bee15c5b2f
Author: G. Branden Robinson <g.branden.robinson@gmail.com>
Date:   Mon Jul 15 09:51:19 2024 -0500

    XXX stdckdint hvunits.h vunits operator + (5/x)

diff --git a/src/roff/troff/hvunits.h b/src/roff/troff/hvunits.h
index 1e3f860e2..7a03eedce 100644
--- a/src/roff/troff/hvunits.h
+++ b/src/roff/troff/hvunits.h
@@ -106,7 +106,8 @@ inline vunits operator +(const vunits & x, const vunits &
y)
 {
   vunits r;
   r = x;
-  r.n += y.n;
+  if (ckd_add(&r.n, r.n, y.n))
+    warning(WARN_RANGE, "integer addition saturated");
   return r;
 }
 

commit 1834d2a1dd5340e5233ac1d1f7b5133a79003523
Author: G. Branden Robinson <g.branden.robinson@gmail.com>
Date:   Mon Jul 15 09:54:05 2024 -0500

    XXX stdckdint hvunits.h vunits operator * (6/x)

diff --git a/src/roff/troff/hvunits.h b/src/roff/troff/hvunits.h
index 7a03eedce..f6a216e62 100644
--- a/src/roff/troff/hvunits.h
+++ b/src/roff/troff/hvunits.h
@@ -146,7 +146,8 @@ inline vunits operator *(const vunits & x, int n)
 {
   vunits r;
   r = x;
-  r.n *= n;
+  if (ckd_mul(&r.n, x.n, n))
+    warning(WARN_RANGE, "integer multiplication saturated");
   return r;
 }
 
@@ -154,7 +155,8 @@ inline vunits operator *(int n, const vunits & x)
 {
   vunits r;
   r = x;
-  r.n *= n;
+  if (ckd_mul(&r.n, x.n, n))
+    warning(WARN_RANGE, "integer multiplication saturated");
   return r;
 }
 

commit 9c11619aea027ec87c46f66311354568bad35b66
Author: G. Branden Robinson <g.branden.robinson@gmail.com>
Date:   Mon Jul 15 09:59:55 2024 -0500

    XXX stdckdint hvunits.h, number.cpp get_vunits (7/x)

diff --git a/src/roff/troff/hvunits.h b/src/roff/troff/hvunits.h
index f6a216e62..9d32e300c 100644
--- a/src/roff/troff/hvunits.h
+++ b/src/roff/troff/hvunits.h
@@ -47,6 +47,7 @@ public:
   friend inline bool operator >=(const vunits&, const vunits&);
   friend inline bool operator ==(const vunits&, const vunits&);
   friend inline bool operator !=(const vunits&, const vunits&);
+  friend bool get_vunits(vunits *, unsigned char, vunits);
 };
 
 extern const vunits V0;
diff --git a/src/roff/troff/number.cpp b/src/roff/troff/number.cpp
index 66bb62cd2..e43340144 100644
--- a/src/roff/troff/number.cpp
+++ b/src/roff/troff/number.cpp
@@ -125,10 +125,12 @@ bool get_vunits(vunits *res, unsigned char si, vunits
prev_value)
     *res = v;
     break;
   case INCREMENT:
-    *res = prev_value + v;
+    if (ckd_add(&(res->n), prev_value.n, v))
+      warning(WARN_RANGE, "integer addition saturated");
     break;
   case DECREMENT:
-    *res = prev_value - v;
+    if (ckd_sub(&(res->n), prev_value.n, v))
+      warning(WARN_RANGE, "integer subtraction saturated");
     break;
   default:
     assert(0 == "unhandled case returned by get_incr_number()");

commit 1a195e791f7131bfb7a8574c40f70e83a59b68c8
Author: G. Branden Robinson <g.branden.robinson@gmail.com>
Date:   Mon Jul 15 10:21:16 2024 -0500

    XXX stdckdint hvunits.h hunits::to_units (8/x)

diff --git a/src/roff/troff/hvunits.h b/src/roff/troff/hvunits.h
index 9d32e300c..da036012c 100644
--- a/src/roff/troff/hvunits.h
+++ b/src/roff/troff/hvunits.h
@@ -209,7 +209,10 @@ inline hunits:: hunits() : n(0)
 
 inline units hunits::to_units()
 {
-  return n * hresolution;
+  units r;
+  if (ckd_mul(&r, n, hresolution))
+    warning(WARN_RANGE, "integer multiplication saturated");
+  return r;
 }
 
 inline bool hunits::is_zero()

commit 31dcea2c15c355a47672d345fe25cf2fe81c2981
Author: G. Branden Robinson <g.branden.robinson@gmail.com>
Date:   Mon Jul 15 10:07:51 2024 -0500

    XXX stdckdint hvunits.h hunits operator * (9/x)

diff --git a/src/roff/troff/hvunits.h b/src/roff/troff/hvunits.h
index da036012c..ff554a913 100644
--- a/src/roff/troff/hvunits.h
+++ b/src/roff/troff/hvunits.h
@@ -261,7 +261,8 @@ inline hunits operator *(const hunits & x, int n)
 {
   hunits r;
   r = x;
-  r.n *= n;
+  if (ckd_mul(&r.n, x.n, n))
+    warning(WARN_RANGE, "integer multiplication saturated");
   return r;
 }
 
@@ -269,7 +270,8 @@ inline hunits operator *(int n, const hunits & x)
 {
   hunits r;
   r = x;
-  r.n *= n;
+  if (ckd_mul(&r.n, x.n, n))
+    warning(WARN_RANGE, "integer multiplication saturated");
   return r;
 }
 

commit c27407c9ba78173fa18454004b66075b54cdaf07
Author: G. Branden Robinson <g.branden.robinson@gmail.com>
Date:   Mon Jul 15 10:41:34 2024 -0500

    XXX stdckdint hvunits.h hunits operator - (10/x)

diff --git a/src/roff/troff/hvunits.h b/src/roff/troff/hvunits.h
index ff554a913..dce32201f 100644
--- a/src/roff/troff/hvunits.h
+++ b/src/roff/troff/hvunits.h
@@ -232,7 +232,8 @@ inline hunits operator -(const hunits & x, const hunits &
y)
 {
   hunits r;
   r = x;
-  r.n -= y.n;
+  if (ckd_sub(&r.n, r.n, y.n))
+    warning(WARN_RANGE, "integer subtraction saturated");
   return r;
 }
 
@@ -240,7 +241,9 @@ inline hunits operator -(const hunits & x)
 {
   hunits r;
   r = x;
-  r.n = -x.n;
+  // Why?  Consider -(INT_MIN) in two's complement.
+  if (ckd_mul(&r.n, x.n, -1))
+    warning(WARN_RANGE, "integer multiplication saturated");
   return r;
 }
 

commit 6071c70662b4948982a493fe6c14b6fb7e73674c
Author: G. Branden Robinson <g.branden.robinson@gmail.com>
Date:   Mon Jul 15 10:46:28 2024 -0500

    XXX stdckdint hvunits.h vunits operator + (11/x)

diff --git a/src/roff/troff/hvunits.h b/src/roff/troff/hvunits.h
index dce32201f..95e644c2a 100644
--- a/src/roff/troff/hvunits.h
+++ b/src/roff/troff/hvunits.h
@@ -224,7 +224,8 @@ inline hunits operator +(const hunits & x, const hunits &
y)
 {
   hunits r;
   r = x;
-  r.n += y.n;
+  if (ckd_add(&r.n, r.n, y.n))
+    warning(WARN_RANGE, "integer addition saturated");
   return r;
 }
 




    _______________________________________________________

Reply to this item at:

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

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

signature.asc
Description: PGP signature