From a8cedd2f91d4d17cd05aaaa816f19e31d2de27aa Mon Sep 17 00:00:00 2001
From: Misa <infoteddyatgitgud@cock.li>
Date: Mon, 15 Jun 2020 14:20:37 -0700
Subject: [PATCH] Prevent out-of-bounds indexing with trinket/crewmate IDs

If a trinket or crewmate ID is out-of-bounds, it will not be created.
This is not only because the `collect`/`customcollect` check in
entityclass::createentity() would then be out-of-bounds, but also
touching it would also be out-of-bounds, too.

Display trinkets will always be created if the ID is out-of-bounds.
Apparently some people (@AllyTally) have been creating display trinkets
with IDs of -1 in order to get a display trinket that always shows up,
which is rather horrifying. But it makes sense, there's a lot more
nonzero int values than there are the amount of int values that are
zero, so it's fairly likely that the `collect` check will always come up
to be true (nonzero). Also, it's more useful to be able to have a
display trinket that always shows up without having to collect a trinket
beforehand, than it is to have it not be created (because technically by
default, you're already in the world where you don't have it created).

Display trinkets still have their `para` set to their ID, though, and if
they managed to gain an `onentity` of 1, bad things could happen... So
just to be sure, I added INBOUNDS checks to crewmates and trinkets in
entityclass::updateentities() so no UB will happen if you collect a
crewmate/trinket with an out-of-bounds ID. Also, I de-duplicated the
`collect`/`customcollect` setting, too.
---
 desktop_version/src/Entity.cpp | 20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/desktop_version/src/Entity.cpp b/desktop_version/src/Entity.cpp
index 3ec4d7d9..4cdbf91b 100644
--- a/desktop_version/src/Entity.cpp
+++ b/desktop_version/src/Entity.cpp
@@ -1399,7 +1399,7 @@ void entityclass::createentity( float xp, float yp, int t, float vx /*= 0*/, flo
 
         //Check if it's already been collected
         entity.para = vx;
-        if (collect[vx]) return;
+        if (!INBOUNDS(vx, collect) || collect[vx]) return;
         break;
     case 9: //Something Shiny
         entity.rule = 3;
@@ -1638,7 +1638,7 @@ void entityclass::createentity( float xp, float yp, int t, float vx /*= 0*/, flo
 
         //Check if it's already been collected
         entity.para = vx;
-        if (!collect[ (vx)]) return;
+        if (INBOUNDS(vx, collect) && !collect[ (vx)]) return;
         break;
     case 23: //SWN Enemies
         //Given a different behavior, these enemies are especially for SWN mode and disappear outside the screen.
@@ -1989,7 +1989,7 @@ void entityclass::createentity( float xp, float yp, int t, float vx /*= 0*/, flo
 
         //Check if it's already been collected
         entity.para = vx;
-        if (customcollect[vx]) return;
+        if (!INBOUNDS(vx, customcollect) || customcollect[vx]) return;
         break;
       case 56: //Custom enemy
         entity.rule = 1;
@@ -2538,9 +2538,13 @@ bool entityclass::updateentities( int i )
             //wait for collision
             if (entities[i].state == 1)
             {
-                if (game.intimetrial)
+                if (INBOUNDS(entities[i].para, collect))
                 {
                     collect[entities[i].para] = true;
+                }
+
+                if (game.intimetrial)
+                {
                     music.playef(25);
                 }
                 else
@@ -2548,7 +2552,6 @@ bool entityclass::updateentities( int i )
                     game.state = 1000;
                     if(music.currentsong!=-1) music.silencedasmusik();
                     music.playef(3);
-                    collect[entities[i].para] = true;
                     if (game.trinkets() > game.stat_trinkets && !map.custommode)
                     {
                         game.stat_trinkets = game.trinkets();
@@ -3168,9 +3171,13 @@ bool entityclass::updateentities( int i )
             }
             else if (entities[i].state == 1)
             {
-                if (game.intimetrial)
+                if (INBOUNDS(entities[i].para, customcollect))
                 {
                     customcollect[entities[i].para] = true;
+                }
+
+                if (game.intimetrial)
+                {
                     music.playef(27);
                 }
                 else
@@ -3179,7 +3186,6 @@ bool entityclass::updateentities( int i )
                     //music.haltdasmusik();
                     if(music.currentsong!=-1) music.silencedasmusik();
                     music.playef(27);
-                    customcollect[entities[i].para] = true;
                 }
 
                 return removeentity(i);