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

[GEN][ZH] Fix uninitialized memory access in Get_OS_Info #438

Merged
merged 3 commits into from
Mar 29, 2025

Conversation

JAJames
Copy link

@JAJames JAJames commented Mar 17, 2025

Changes below are minimal

Analysis:

  • generals.exe was crashing in CPUDetectClass::Init_Compact_Log when attempting to log the Code and SubCode strings on os_info, due to them being initialized with garbage data.
  • This was caused because there were not entries for any Windows newer than XP
  • This was also caused because the os_info object was not default-initialized with any sane values

Changes:

  1. Moved the zero-initialization from the default block, to just always execute first instead, so that it's guaranteed to be zero-initialized and set
  2. Added entries for Windows Vista/7/8/8.1/10/11
  3. Added a fallback entry for versions otherwise not detected
  4. My IDE ate the micro symbol, I suppose because the source file isn't actually stored as UTF-8, but some other encoding instead. Instead of dwelling on it, I just replaced it with the letter u.

Impact:

  • Startup no longer crashes here (instead, I now crash due to an INI parsing issue)
  • I don't think this actually affects anything else however, except for logs.

@xezon xezon added Bug Something is not working right Critical Severity: Minor < Major < Critical < Blocker labels Mar 17, 2025
@DevGeniusCode DevGeniusCode added Generals Related Generals only ZeroHour Relates to Zero Hour labels Mar 17, 2025
@VodkaPotato
Copy link

VodkaPotato commented Mar 25, 2025

Be advised CPUDetectClass::Init_OS on Win10 will initialize that we have version Major 6 and minor 2, and therefore we get windows 8.0 as the reported OS!
image
image

I tested and this fixes the crash, and I don't think it currently matters but revisiting this we might want look into what GetVersionEx does to get those numbers...

@xezon xezon changed the title #437: Fix bad memory access [GEN][ZH] Fix uninitialized memory access in Get_OS_Info Mar 29, 2025
@xezon xezon self-assigned this Mar 29, 2025
@xezon
Copy link

xezon commented Mar 29, 2025

Be advised CPUDetectClass::Init_OS on Win10 will initialize that we have version Major 6 and minor 2

I can confirm this also happens on my machine.

Other than that, fix works correctly and prevents crashing when the Options.ini is absent.

@xezon xezon merged commit 7f4c703 into TheSuperHackers:main Mar 29, 2025
17 checks passed
@xezon xezon added this to the Code foundation build up milestone Mar 31, 2025
@xezon xezon added the Stability Concerns stability of the runtime label Apr 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is not working right Critical Severity: Minor < Major < Critical < Blocker Generals Related Generals only Stability Concerns stability of the runtime ZeroHour Relates to Zero Hour
Projects
None yet
5 participants