MathewWi / wiidoom

Automatically exported from code.google.com/p/wiidoom
0 stars 0 forks source link

Overflow bug causes load game crash in Wiidoom #77

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?
1.  Compile WiiDoom with devkitPPC r26, or one of the recent clones such
    as: https://code.google.com/r/benshadwick-wiidoom/source/checkout
2.  Play and save a game.
3.  Load the game.

What is the expected output? What do you see instead?

DSI code dump.

Please provide any additional information below.

SOLUTION:

I think this may be an old bug that has been there since the beginning.

The g_game.c file uses a savebuffer pointer to point to the allocated memory 
space of the loaded save game file. Something was stomping on the first byte of 
the pointer -- not what it pointed to, but the address in the pointer variable 
itself. So, in my case savebuffer started out containing an address value of 
0x809E7C78, but by the time Z_Free was called on it to free the memory, the 
value was 0x009E7C78. Something zeroed out the first byte, and the pointer no 
longer pointed to valid memory.

The problem seems to be caused by a memset of mousebuttons. In the declarations 
in g_game.c:

static bool mousearray[4];
static bool *mousebuttons = &mousearray[1]; // allow [-1]

Notice that mousebuttons points to one position in of offset into mousearray. 
But in G_DoLoadLevel(), it zeros it out with a memset:

memset (mousebuttons, 0, sizeof(mousebuttons));

This memset clears 4 bytes of memory, and because it is starting one byte in to 
the 4-byte mousearray, it overwrites the first byte of whatever field follows 
mousearray. Looking at the linker map, in my case, that was the savebuffer 
pointer.

The memset should clear out the mousearray itself and not the mousebuttons 
pointer to it (and joyarray, which is a similar arrangement, but won't overflow 
because the array length is 13):

memset (mousearray, 0, sizeof(mousearray));
memset (joyarray, 0, sizeof(joyarray));

Original issue reported on code.google.com by jo...@earthlink.net on 7 Sep 2013 at 2:22