Skip to content

Conversation

@cawtds
Copy link
Contributor

@cawtds cawtds commented Apr 25, 2025

Description

The first call of CheckMoveLimitations each battle (except for the first one each load) is done with gBattlersCount from the previous battle, which is either 2 or 4 and in most cases OOB.
Since the first call after loading the game is always done with 0, setting gActiveBattler = 0 is presumably safe.
oob

Discord contact info

.cawt

@GriffinRichards
Copy link
Member

Most of the battler arrays are explicitly sized to MAX_BATTLERS_COUNT, do you have an example of where it can go out of bounds? That's not to say reading those elements is correct, but it'd be helpful to clarify what's actually going wrong here. If gBattlersCount is incorrect at the start of a battle why is that left unchanged?

@cawtds
Copy link
Contributor Author

cawtds commented Dec 10, 2025

SetUpBattleVarsAndBirchZigzagoon -> BattleAI_HandleItemUseBeforeAISetup -> BattleAI_SetupAIData ->. moveLimitations = CheckMoveLimitations(gActiveBattler, 0, MOVE_LIMITATIONS_ALL).
At this point gActiveBattler has not been set yet, so it contains the last value of the previous battle, which is either 0 (after a reset), 2 (single battle) or 4 (double battle) due to gBattlersCount being the for-loop limit. There is multiple accesses of gBattleMons which is of size 4 with battler, so the first call of CheckMoveLimitations after a double battle has OOB reads.

@GriffinRichards GriffinRichards merged commit b4abc6b into pret:master Jan 3, 2026
1 check passed
@cawtds cawtds deleted the fix-active-battler-ub branch January 3, 2026 20:27
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

Successfully merging this pull request may close these issues.

2 participants