Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OoBs memory access bug in FileChoose_LoadGame #1130

Open
mzxrules opened this issue Feb 3, 2022 · 1 comment
Open

OoBs memory access bug in FileChoose_LoadGame #1130

mzxrules opened this issue Feb 3, 2022 · 1 comment

Comments

@mzxrules
Copy link
Contributor

mzxrules commented Feb 3, 2022

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
Copy link
Collaborator

To reword this issue:

Consider this block of code

if ((gSaveContext.save.info.equips.buttonItems[0] != ITEM_SWORD_KOKIRI) &&
(gSaveContext.save.info.equips.buttonItems[0] != ITEM_SWORD_MASTER) &&
(gSaveContext.save.info.equips.buttonItems[0] != ITEM_SWORD_BIGGORON) &&
(gSaveContext.save.info.equips.buttonItems[0] != ITEM_GIANTS_KNIFE)) {
gSaveContext.save.info.equips.buttonItems[0] = ITEM_NONE;
swordEquipValue =
(gEquipMasks[EQUIP_TYPE_SWORD] & gSaveContext.save.info.equips.equipment) >> (EQUIP_TYPE_SWORD * 4);
gSaveContext.save.info.equips.equipment &= gEquipNegMasks[EQUIP_TYPE_SWORD];
gSaveContext.save.info.inventory.equipment ^= OWNED_EQUIP_FLAG(EQUIP_TYPE_SWORD, swordEquipValue - 1);
}

it is in function
void FileSelect_LoadGame(GameState* thisx) {

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)

swordEquipValue =
(gEquipMasks[EQUIP_TYPE_SWORD] & gSaveContext.save.info.equips.equipment) >> (EQUIP_TYPE_SWORD * 4);

then on

gSaveContext.save.info.inventory.equipment ^= OWNED_EQUIP_FLAG(EQUIP_TYPE_SWORD, swordEquipValue - 1);

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)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants