Out of bounds crashes on snake blocks, creepers, and move blocks

[calzoneman]

I'm not sure exactly why this happens, but certain levels try to call DrawMoveBlock, DrawSnake, and DrawCrp with out of bounds EX resulting in a segfault.

Example backtraces:

(gdb) r
Starting program: bin/toost -p mm2-bcds/41114184.bcd -o /tmp/o.png -s /tmp/s.png
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib64/libthread_db.so.1".
Done parsing 160アスレチック
Cairo PNG writing took 22 milliseconds
Rendering overworld took 59 milliseconds
Done parsing 160アスレチック

Program received signal SIGSEGV, Segmentation fault.
0x00005555555c2b55 in LevelDrawer::DrawSnake (this=this@entry=0x555557aa29b0, EX=2 '\002', X=X@entry=10480, 
    Y=Y@entry=2800, SW=5, SH=<optimized out>) at src/LevelDrawer.cpp:569
569                     switch(level.MapSnk[EX - 1].Node[0].Dir) {
(gdb) bt
#0  0x00005555555c2b55 in LevelDrawer::DrawSnake (this=this@entry=0x555557aa29b0, EX=2 '\002', 
    X=X@entry=10480, Y=Y@entry=2800, SW=5, SH=<optimized out>) at src/LevelDrawer.cpp:569
#1  0x00005555555c9768 in LevelDrawer::DrawItem (this=this@entry=0x555557aa29b0, 
    K=std::unordered_set with 4 elements = {...}, L=L@entry=false) at src/LevelDrawer.cpp:1574
#2  0x0000555555563e16 in DrawMap (level=level@entry=0x55555798f6c0, isOverworld=isOverworld@entry=false, 
    log=log@entry=false, destination="/tmp/s.png") at src/main.cpp:176
#3  0x0000555555565b8e in AttemptRender (choice="mm2-bcds/41114184.bcd", 
    log=log@entry=false, destinationOverworld="/tmp/o.png", destinationSubworld="/tmp/s.png")
    at src/main.cpp:369
#4  0x000055555556ad1f in main (argc=<optimized out>, argv=<optimized out>) at src/main.cpp:994

(gdb) r
Starting program: bin/toost -p mm2-bcds/40675457.bcd -o /tmp/o.png -s /tmp/s.png
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib64/libthread_db.so.1".
Done parsing Builder kaizo

Program received signal SIGSEGV, Segmentation fault.
0x00005555555c2758 in LevelDrawer::DrawCrp (this=this@entry=0x555557b659f0, EX=<optimized out>,
    X=X@entry=5760, Y=Y@entry=3120) at src/LevelDrawer.cpp:453
453             switch(level.MapCrp[EX - 1].Node[0]) {
(gdb) bt
#0  0x00005555555c2758 in LevelDrawer::DrawCrp (this=this@entry=0x555557b659f0, EX=<optimized out>,
    X=X@entry=5760, Y=Y@entry=3120) at src/LevelDrawer.cpp:453
#1  0x00005555555cb8c3 in LevelDrawer::DrawItem (this=this@entry=0x555557b659f0,
    K=std::unordered_set with 2 elements = {...}, L=L@entry=false) at src/LevelDrawer.cpp:2034
#2  0x0000555555563d08 in DrawMap (level=level@entry=0x55555575f5f0, isOverworld=isOverworld@entry=true,
    log=log@entry=false, destination="/tmp/o.png") at src/main.cpp:162
#3  0x0000555555565ace in AttemptRender (choice="mm2-bcds/40675457.bcd",
    log=log@entry=false, destinationOverworld="/tmp/o.png", destinationSubworld="/tmp/s.png")
    at src/main.cpp:358
#4  0x000055555556ad1f in main (argc=<optimized out>, argv=<optimized out>) at src/main.cpp:994

(gdb) r
Starting program: bin/toost -p mm2-bcds/35526829.bcd -o /tmp/o.png -s /tmp/s.png
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib64/libthread_db.so.1".
Done parsing Forteresse de Spike
Cairo PNG writing took 46 milliseconds
Rendering overworld took 67 milliseconds
Done parsing Forteresse de Spike

Program received signal SIGSEGV, Segmentation fault.
bt0x00005555555c30dd in LevelDrawer::DrawMoveBlock (this=this@entry=0x555557b48460, ID=ID@entry=85 'U',
    EX=<optimized out>, X=X@entry=25280, Y=Y@entry=80) at src/LevelDrawer.cpp:733
733                     switch(level.MapTrackBlk[EX - 1].Node[0].p1) {
(gdb) bt
#0  0x00005555555c30dd in LevelDrawer::DrawMoveBlock (this=this@entry=0x555557b48460, ID=ID@entry=85 'U',
    EX=<optimized out>, X=X@entry=25280, Y=Y@entry=80) at src/LevelDrawer.cpp:733
#1  0x00005555555c7db8 in LevelDrawer::DrawItem (this=this@entry=0x555557b48460,
    K=std::unordered_set with 2 elements = {...}, L=L@entry=false) at src/LevelDrawer.cpp:1361
#2  0x0000555555563e94 in DrawMap (level=level@entry=0x555557b03ea0, isOverworld=isOverworld@entry=false,
    log=log@entry=false, destination="/tmp/s.png") at src/main.cpp:178
#3  0x0000555555565bae in AttemptRender (choice="mm2-bcds/35526829.bcd",
    log=log@entry=false, destinationOverworld="/tmp/o.png", destinationSubworld="/tmp/s.png")
    at src/main.cpp:369
#4  0x000055555556ad3f in main (argc=<optimized out>, argv=<optimized out>) at src/main.cpp:994

The following diff avoids the problem for me locally by skipping out-of-bounds draws, although I haven't verified whether this results in anything missing from the image that exists in game. It probably bears further investigation why the DrawItem function is calling these functions with the invalid parameter in the first place.

diff --git a/src/LevelDrawer.cpp b/src/LevelDrawer.cpp
index fc92c4f..b03b510 100644
--- a/src/LevelDrawer.cpp
+++ b/src/LevelDrawer.cpp
@@ -1,4 +1,5 @@
 #include "LevelDrawer.hpp"
+#include <iostream>
 
 void LevelDrawer::Setup() {
 	level.TileLoc[4][0]   = Point(1, 0);
@@ -440,6 +441,11 @@ void LevelDrawer::DrawImageRotateOpacity(
 }
 
 void LevelDrawer::DrawCrp(unsigned char EX, int X, int Y) {
+	if (EX > level.MapCrp.size()) {
+		std::cout << "warning: out of bounds creeper draw EX=" << (int)EX << std::endl;
+		return;
+	}
+
 	int XX = std::round(X / 160.0 + 1);
 	int YY = std::round((Y + 80) / 160.0 + 1);
 	int i  = 0;
@@ -559,6 +565,11 @@ void LevelDrawer::DrawCrp(unsigned char EX, int X, int Y) {
 }
 
 void LevelDrawer::DrawSnake(unsigned char EX, int X, int Y, int SW, int SH) {
+	if (EX > level.MapSnk.size()) {
+		std::cout << "warning: out of bounds snake draw EX=" << (int)EX << std::endl;
+		return;
+	}
+
 	int XX = 0;
 	int YY = 0;
 
@@ -566,6 +577,7 @@ void LevelDrawer::DrawSnake(unsigned char EX, int X, int Y, int SW, int SH) {
 	if(EX < 0x10) {
 		XX = std::round((X + SW * 80) / 160.0);
 		EX = (unsigned char)(EX % 0x10);
+
 		switch(level.MapSnk[EX - 1].Node[0].Dir) {
 		case 1:
 		case 5:
@@ -712,7 +724,7 @@ void LevelDrawer::DrawMoveBlock(unsigned char ID, unsigned char EX, int X, int Y
 	YY     = std::round((Y + 80) / 160.0 + 1);
 	int i  = 0;
 
-	if(EX > level.MapMoveBlk.size()) {
+	if(EX > level.MapMoveBlk.size() || EX > level.MapTrackBlk.size()) {
 		return;
 	}

[calzoneman]

I just realized the >= should be a > in my diff because the subtraction of 1 already occurs when using it. (fixed above)