search

SkyTemple / ExplorerScript

ExplorerScript and SSBScript: Script languages for decompiled SSB (Pokémon Mystery Dungeon Explorers of Sky)
MIT License
16 stars 6 forks source link

Add range check for WaitExecute opcodes #20

Closed Frostbyte0x70 closed 2 years ago

Frostbyte0x70 commented 2 years ago

Originally reported at https://discord.com/channels/710190644152369162/724746586072023182/1002499328184692807

Seems like SkyTemple allows specifying out-of-bounds IDs to WaitExecute commands (in particular, an actor ID where an object ID is expected and vice-versa). The script compiles and works, probably because actors and objects are read the same way by the game. However, trying to reopen a script with out-of-bounds IDs in SkyTemple causes an error.

It would be a good idea to add some validation to the parameters of these opcodes (and also to other opcodes that might take entity IDs that are supposed to be in a certain range, if there's any) to prevent this.

Frostbyte0x70 commented 2 years ago

Update (https://discord.com/channels/710190644152369162/724746586072023182/1002716669149462609): The way SkyTemple handles this seems to be correct. The suggested fix is to replace the parameter with the numeric ID when decompiling if the ID isn't in the actor list. The fix should be applied around https://github.com/SkyTemple/skytemple-files/blob/10561986c27eefa206f4b41a8131ffc0e871b2ff/skytemple_files/script/ssb/model.py#L315.

theCapypara commented 2 years ago

This will probably be fixed by https://github.com/SkyTemple/skytemple-files/pull/275