bug-groff
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[bug #64229] [troff] want diagnostic for diversion length overflow


From: G. Branden Robinson
Subject: [bug #64229] [troff] want diagnostic for diversion length overflow
Date: Wed, 14 Aug 2024 07:06:12 -0400 (EDT)

Update of bug #64229 (group groff):

                Severity:           4 - Important => 3 - Normal             
              Item Group:      Crash/Unresponsive => Warning/Suspicious
behaviour
                  Status:                    None => In Progress            
             Assigned to:                    None => gbranden               
                 Summary: [troff] I haz a DoS attack => [troff] want
diagnostic for diversion length overflow

    _______________________________________________________

Follow-up Comment #6:

Dave's instincts in comment #3 were pretty good.

I instrumented `macro_diversion::output()` in "div.cpp" and learned that the
Linux OOM killer slays the DoS attack proposed in comment #0 at only about 4
million lines of output on my machine.

While my box isn't supremely specced, that's still 3 orders or magnitude less
than INT_MAX, which is the effective maximum "page" length of a diversion.

So I think we can get by with a check for diversion length overflow, and emit
a fatal error if that overflow occurs.

Changing Item Group and kicking down to normal severity.  


diff --git a/src/roff/troff/div.cpp b/src/roff/troff/div.cpp
index 772540c09..fc7c16570 100644
--- a/src/roff/troff/div.cpp
+++ b/src/roff/troff/div.cpp
@@ -241,6 +241,8 @@ macro_diversion::~macro_diversion()
   dn_reg_contents = vertical_position.to_units();
 }
 
+static int DIVERSION_LENGTH_MAX = INT_MAX;
+
 vunits macro_diversion::distance_to_next_trap()
 {
   vunits distance = 0;
@@ -250,7 +252,8 @@ vunits macro_diversion::distance_to_next_trap()
   else
     // Do the (saturating) arithmetic ourselves to avoid an error
     // diagnostic from constructor in number.cpp.
-    distance = units(INT_MAX / vresolution);
+    //distance = units(INT_MAX / vresolution);
+    distance = units(DIVERSION_LENGTH_MAX / vresolution);
   assert(distance >= 0);
   return distance;
 }
@@ -300,6 +303,17 @@ void macro_diversion::output(node *nd, int retain_size,
   if (width > max_width)
     max_width = width;
   vunits x = v.pre + v.pre_extra + v.post + v.post_extra;
+  int new_vpos = 0;
+  int vpos = vertical_position.to_units();
+  int lineht = x.to_units();
+  bool overflow = false;
+  if (ckd_add(&new_vpos, vpos, lineht))
+    overflow = true;
+  else if (new_vpos > DIVERSION_LENGTH_MAX)
+    overflow = true;
+  if (overflow)
+    fatal("diversion overflow (vertical position: %1u,"
+         " next line height: %2u)", vpos, lineht);
   if (vertical_position_traps_flag
       && !diversion_trap.is_null() && diversion_trap_pos > vertical_position
       && diversion_trap_pos <= vertical_position + x) {


While I'm at it I'm parameterizing the maximum diversion length, as seen
above, instead of using `INT_MAX` as a magic number. 


    _______________________________________________________

Reply to this item at:

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

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

Attachment: signature.asc
Description: PGP signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]