oot icon indicating copy to clipboard operation
oot copied to clipboard

OoBs memory access bug in FileChoose_LoadGame

Open mzxrules opened this issue 3 years ago • 1 comments

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

mzxrules avatar Feb 03 '22 21:02 mzxrules

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)

Dragorn421 avatar Sep 19 '23 19:09 Dragorn421