Skip to content
This repository has been archived by the owner on Jul 18, 2023. It is now read-only.

Fix null dereference #1

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Sirius902
Copy link

@Sirius902 Sirius902 commented Aug 7, 2021

It is actually possible for the pointer to CurSubmenu to be null. It's better to explicitly handle this instead of getting an undefined or possibly garbage value from reading from address 0.

Edit: Is zero an okay value for CurSubmenu if the pointer was null? That's what it'll be in this PR how it is now.

@1234567890num
Copy link
Owner

What kind of situation would cause the pointer to CurSubmenu to be null?

@Sirius902
Copy link
Author

Sirius902 commented Aug 13, 2021

Seems like it's any time you're not in a sub-menu of the command menu. For example if you're hovering over any option in the base command menu like Attack, Magic, or Party then it'll be null but if you click on Magic and get the list of spells to show up then it won't be null.

Edit: The only reason it doesn't it doesn't do anything unexpected atm is because I return 0 from the read functions in the backend if you read from a bad address but in theory you could get any garbage value back so I wouldn't rely on it (if I didn't specifically code in that behavior). Maybe it would be more appropriate to return nil if someone dereferences an address with no access like 0?

@1234567890num
Copy link
Owner

I don't know how LuaBackend accesses stuff. What addresses are possible? Who's to say that address 0 won't ever be looked up?

Is ReadPointer still available in your LuaBackend? I might be thinking of combining the conditionals for platforms using the new function argument for read/write.

@Sirius902
Copy link
Author

Sirius902 commented Aug 16, 2021

You could try to read or write to address 0 in backend but since Windows has memory protections on the zero page (a region of memory starting at address 0) there's nothing meaningful there to access (there can't be any game-related data there). Technically actually reading or writing something to that address will cause an Access Violation but since I check to make sure you actually have access to the memory region (and if possible change the permissions to allow reading / writing to that address) I pretty much ignore reading / writing to address 0 to avoid crashing the game.

Yes ReadPointer is still available, don't really plan on getting rid of it.

Edit: You're probably not going to have access to any memory region the game or some library it depends on doesn't use.

Oh also do note that something like ReadByte(0) doesn't actually read from address 0 since it's not an absolute access and starts from the address of the KH2 module in memory plus some offset. But since you did GetLong on a null pointer and then read from that address you're accessing address 0.

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

Successfully merging this pull request may close these issues.

2 participants