zeldaret / oot

Decompilation of The Legend of Zelda: Ocarina of Time
4.83k stars 593 forks source link

OoBs memory access bug in FileChoose_LoadGame #1130

Open mzxrules opened 2 years ago

mzxrules commented 2 years ago

https://github.com/zeldaret/oot/blob/master/src/overlays/gamestates/ovl_file_choose/z_file_choose.c#L1501

If swordEquipMask is 0, an out of bounds memory access will occur. This naturally happens when loading a new game, as the player's B button isn't assigned to a sword item, and gSaveContext.equips.equipment's sword bits are 0. The only reason odd behavior doesn't occur on N64 is that the data at index -1 is either a null pointer or padding, effectively becoming a no op

Dragorn421 commented 1 year ago

To reword this issue:

Consider this block of code https://github.com/zeldaret/oot/blob/185c9cbf1aed3190f335fe6745cd220d2e0b2b62/src/overlays/gamestates/ovl_file_choose/z_file_choose.c#L1505-L1515 it is in function https://github.com/zeldaret/oot/blob/185c9cbf1aed3190f335fe6745cd220d2e0b2b62/src/overlays/gamestates/ovl_file_choose/z_file_choose.c#L1437

This function runs also when a newly created file is loaded, at which point the current b button item is not a sword (it's "no item"). so this condition passes and the block executes

then swordEquipValue gets 0 (EQUIP_VALUE_SWORD_NONE) https://github.com/zeldaret/oot/blob/185c9cbf1aed3190f335fe6745cd220d2e0b2b62/src/overlays/gamestates/ovl_file_choose/z_file_choose.c#L1511-L1512

then on https://github.com/zeldaret/oot/blob/185c9cbf1aed3190f335fe6745cd220d2e0b2b62/src/overlays/gamestates/ovl_file_choose/z_file_choose.c#L1514 the right hand side expands to (gBitFlags[swordEquipValue - 1] << gEquipShifts[EQUIP_TYPE_SWORD]) which uses gBitFlags[-1]

and that's an OOB access which apparently turns out to be fine (must load a 0 I guess)