Skip to content

Add "ctypes-based" fallback to the hashing by reading the non-writable sections of the running game#85

Merged
monkeyman192 merged 15 commits intomonkeyman192:masterfrom
samjviana:master
Sep 30, 2025
Merged

Add "ctypes-based" fallback to the hashing by reading the non-writable sections of the running game#85
monkeyman192 merged 15 commits intomonkeyman192:masterfrom
samjviana:master

Conversation

@samjviana
Copy link
Contributor

@samjviana samjviana commented Sep 19, 2025

I wrote this mainly to help GamePass players (like myself) who don' t want to go through the trouble of making a forced copy of the game files out of Windows default installation directoty ... but this fallback can also help with any permission errors that might happen while opening the game file.
Only downside would be the fact that the game (obviously) needs to be running and that this is windows-only.

One specific thing that I could add is that maybe instead of raising errors from this function you might want to just return None and keep pyMHF running as it seems to be optional at this point.



# ctypes/wintypes/kernel32/psapi struct definitions
class MODULEINFO(ctypes.Structure):
Copy link
Owner

@monkeyman192 monkeyman192 Sep 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be good to move things like this into pymhf/utils/winapi.py.
Some of these functions and such may also be exposed in pymem, so can be used directly from there rather than defining them again ourselves (cf. pymhf/core/caching.py for example where I import MODULEINFO from pymem.

Edit: This includes things like _read_process_memory and _get_page_size. If they don't have implementations in pymem already, they can be moved to the winapi.py file as well with the rest of the stuff that would still need to be defined.
This way this has module can contain just the code relating to the hashing, and we can keep the messiness of the windows api in a separate file which will be easier to maintain.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

totally agree ... i admit i did not look into the whole stack. i'll make this changes right away

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this was a simple change, but i'll leave to sync the pr when i change the logic to work on top of pymem
hopefully i'm not blocking you

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

all good, I have other things I am working on improving so no huge rush 😄

return sys_info.dwPageSize


def hash_bytes_from_memory(binary_path: str, _bufsize: int = 2**18) -> str:
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think a good bit of this function could be simplified if the pymem.Pymem object was passed in instead of the path itself. By the time this function is called we have already found the process and some details about it. No point in going through this whole process again.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you're right ... i'll take a more careful look to integrate it correctly with what already exists of pymem. i just can't right now as it is already past 1am here.
as soon as i get back from work i'll get into this

@samjviana
Copy link
Contributor Author

Just integrated the code to use pymem.Pymem object as well as changed it to use some things already defined in pymem (like read_bytes, MEMORY_PROTECTION, etc).
Also changed the organization in order for it to be a little more readable without the commments.

@monkeyman192
Copy link
Owner

My unit tests clearly don't cover the whole code bases since I can see that there are still some cases where python 3.10+ syntax exists.
I guess it doesn't get checked because the code is in the if TYPE_CHECKING of hashing.py...

I'm also not sure why the CI doesn't fail for the uv run ruff check ./pymhf ./tests because it does when run locally...

return True


def _read_bytes_into(
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suspect this function could be replaced with read_ctype in pymem...

current = address
while current < region_end:
to_read = min(_bufsize, region_end - current)
res = _read_bytes_into(
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could use read_bytes from pymem also

return sys_info.dwPageSize or 4096


def _get_main_module(pm_binary: pymem.Pymem) -> MODULEINFO:
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

functions like this I think could also go in the winapi.py file.
I think for this file we can have probably just the two functions hash_bytes_from_file and hash_bytes_from_memory, and pretty much everything else can go under the other file.

from ctypes import _CData, _Pointer, _SimpleCData

CDataLike: TypeAlias = (
_CData | _SimpleCData | _Pointer[Any] | ctypes.Structure | ctypes.Union | ctypes.Array[Any]
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is the place where it's using 3.10+ syntax...

@samjviana
Copy link
Contributor Author

test was probably ignoring the TYPE_CHEKING scope like u said :/
removed the _read_bytes_into ... now i think we should be good to go. lmk if there are any other changes

@monkeyman192
Copy link
Owner

Looks great to me thanks!
The only thing left to do (which I should have mentioned before (sorry)) is to add a new line to the change log describing the changes made. Just keep it short and simple and also credit yourself in the same way other changes are listed (there are a few other places where there are similar change lines)

@monkeyman192 monkeyman192 merged commit 324cd73 into monkeyman192:master Sep 30, 2025
9 checks passed
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