Bad V_DrawPatch crash when text strings are too long for the screen

[plumsinus]

As reported by Dragonsbretheren at http://www.doomworld.com/vb/post/1301855

An easier test case is with the HacX 1.2 iwad: start a game and type SEEIT (or IDBEHOLD is using -nocheats).

[fabiangreffrath]

Thank you so much for forwarding issues here that were raised in the forums so I don't lose track of them. This is really helpful!

[plumsinus]

No problem!

[plumsinus]

It seems quit messages will still give Bad V_DrawPatch if they are too long for the screen. Other messages I tried were ok: automap names, options text, status messages, etc.

[fabiangreffrath]

Indeed, but there isn't even a check for quit message string lengths in Vanilla.

[plumsinus]

How about this?

--- crispy-doom-git/src/doom/m_menu.c   2014-09-03 06:59:29 -0400
+++ crispy-doom-plm/src/doom/m_menu.c   2014-09-04 06:58:46 -0400
@@ -2334,6 +2334,7 @@
             }

        x = 160 - M_StringWidth(string) / 2;
+        x = (x > 0) ? x : 0;
        M_WriteText(x, y, string);
        y += SHORT(hu_font[0]->height);
    }

[fabiangreffrath]

This could still write over the right screen boundary if the width of the composed text is > 320 px. I'd suggest to not write the text line at all if its width is > 320 px. How about this?

--- a/src/doom/m_menu.c
+++ b/src/doom/m_menu.c
@@ -2334,6 +2334,7 @@ void M_Drawer (void)
             }

            x = 160 - M_StringWidth(string) / 2;
+           if (x >= 0)
            M_WriteText(x, y, string);
            y += SHORT(hu_font[0]->height);
        }

[fabiangreffrath]

Even the (x == 0) case is dangerous, because string width could be 321!

[plumsinus]

This could still write over the right screen boundary if the width of the composed text is > 320 px.

If the string is too long for 320px, the additional patches don't get drawn though, just as with other text strings. The spacing is messed up but that's it. Here's an example where I replaced the E font graphic with 160 px of Es. In any case either solution is fine with me, "not crashing" is the main concern.

[fabiangreffrath]

I see. I did it the Killough way. ;)

--- a/src/doom/m_menu.c
+++ b/src/doom/m_menu.c
@@ -2334,7 +2334,7 @@ void M_Drawer (void)
             }

            x = 160 - M_StringWidth(string) / 2;
-           M_WriteText(x, y, string);
+           M_WriteText(x > 0 ? x : 0, y, string); // [crispy] prevent negative x-coords
            y += SHORT(hu_font[0]->height);
        }