mpo/VVVVVV
1
0
Fork 0
mirror of https://github.com/TerryCavanagh/VVVVVV.git synced 2025-01-10 19:09:45 +01:00
Code Issues Releases Wiki Activity

Add bounds checks to script commands that didn't have them

Browse source 
Continuing from #280, another potential source of out-of-bounds indexing
(and thus, Undefined Behavior badness) comes from script commands. A
majority of them don't do any input validation at all, which means the
potential for out-of-bounds indexing and segfaulting in custom levels.
So it's always good to add bounds checks to them.

Interesting note, the only existing command that has bounds checks is
the flag() command. That means you can't turn out-of-bounds flags on or
off. But there's no bounds checks for ifflag(), or customifflag(), which
means you CAN index out-of-bounds with those commands! That's a bit bad
to do, so.

Also, I decided to add the bounds checks for playef() at the
musicclass::playef() level, instead of just the level of the playef()
command. I don't know of any other cases outside of the command where
musicclass::playef() will index out of bounds, but musicclass is the one
containing the indexed vector anyway, I wanted to cover more cases, and
it's better to be safe than sorry.
This commit is contained in:
Misa 2020-06-12 21:55:41 -07:00 committed by Ethan Lee
parent fdce412680
commit 14afae1a40
2 changed files with 35 additions and 10 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
desktop_version/src/Music.cpp
View file

@ -391,6 +391,10 @@ void musicclass::changemusicarea(int x, int y)
void musicclass::playef(int t)
{
if (t < 0 || t >= (int) soundTracks.size())
{
return;
}
int channel;
channel = Mix_PlayChannel(-1, soundTracks[t].sound, 0);

39
desktop_version/src/Script.cpp
View file

@ -97,12 +97,16 @@ void scriptclass::run()
int temprx=ss_toi(words[1])-1;
int tempry=ss_toi(words[2])-1;
int curlevel=temprx+(ed.maxwidth*(tempry));
bool inbounds = curlevel >= 0 && curlevel < 400;
if (inbounds)
{
ed.level[curlevel].warpdir=ss_toi(words[3]);
}
//If screen warping, then override all that:
graphics.backgrounddrawn = false;
//Do we update our own room?
if(game.roomx-100==temprx && game.roomy-100==tempry){
if(inbounds && game.roomx-100==temprx && game.roomy-100==tempry){
map.warpx=false; map.warpy=false;
if(ed.level[curlevel].warpdir==0){
map.background = 1;
@ -132,7 +136,8 @@ void scriptclass::run()
}
if (words[0] == "ifwarp")
{
if (ed.level[ss_toi(words[1])-1+(ed.maxwidth*(ss_toi(words[2])-1))].warpdir == ss_toi(words[3]))
int room = ss_toi(words[1])-1+(ed.maxwidth*(ss_toi(words[2])-1));
if (room >= 0 && room < 400 && ed.level[room].warpdir == ss_toi(words[3]))
{
load("custom_"+words[4]);
position--;
@ -174,7 +179,8 @@ void scriptclass::run()
}
else if (words[0] == "customifflag")
{
if (obj.flags[ss_toi(words[1])])
int flag = ss_toi(words[1]);
if (flag >= 0 && flag < (int) obj.flags.size() && obj.flags[flag])
{
load("custom_"+words[2]);
position--;
@ -1284,7 +1290,8 @@ void scriptclass::run()
}
else if (words[0] == "ifexplored")
{
if (map.explored[ss_toi(words[1]) + (20 * ss_toi(words[2]))] == 1)
int room = ss_toi(words[1]) + (20 * ss_toi(words[2]));
if (room >= 0 && room < (int) map.explored.size() && map.explored[room] == 1)
{
load(words[3]);
position--;
@ -1308,7 +1315,8 @@ void scriptclass::run()
}
else if (words[0] == "ifflag")
{
if (obj.flags[ss_toi(words[1])])
int flag = ss_toi(words[1]);
if (flag >= 0 && flag < (int) obj.flags.size() && obj.flags[flag])
{
load(words[2]);
position--;
@ -1316,7 +1324,8 @@ void scriptclass::run()
}
else if (words[0] == "ifcrewlost")
{
if (game.crewstats[ss_toi(words[1])]==false)
int crewmate = ss_toi(words[1]);
if (crewmate >= 0 && crewmate < (int) game.crewstats.size() && game.crewstats[crewmate]==false)
{
load(words[2]);
position--;
@ -1340,11 +1349,19 @@ void scriptclass::run()
}
else if (words[0] == "hidecoordinates")
{
map.explored[ss_toi(words[1]) + (20 * ss_toi(words[2]))] = 0;
int room = ss_toi(words[1]) + (20 * ss_toi(words[2]));
if (room >= 0 && room < (int) map.explored.size())
{
map.explored[room] = 0;
}
}
else if (words[0] == "showcoordinates")
{
map.explored[ss_toi(words[1]) + (20 * ss_toi(words[2]))] = 1;
int room = ss_toi(words[1]) + (20 * ss_toi(words[2]));
if (room >= 0 && room < (int) map.explored.size())
{
map.explored[room] = 1;
}
}
else if (words[0] == "hideship")
{
@ -1900,7 +1917,11 @@ void scriptclass::run()
music.haltdasmusik();
music.playef(3);
obj.collect[ss_toi(words[1])] = true;
int trinket = ss_toi(words[1]);
if (trinket >= 0 && trinket < (int) obj.collect.size())
{
obj.collect[trinket] = true;
}
graphics.textboxremovefast();