Initialize Addresses section offsets and fail loudly if a section is missing#143
Open
JustinMDotNet wants to merge 1 commit into
Open
Initialize Addresses section offsets and fail loudly if a section is missing#143JustinMDotNet wants to merge 1 commit into
JustinMDotNet wants to merge 1 commit into
Conversation
The m_codeOffset, m_dataOffset, and m_rdataOffset fields on Addresses had no in-class initializer and no constructor init. LoadSections only wrote them inside matching strcmp branches, with no else and no default. If the host module ever lacks one of .text, .data, or .rdata (packed exes, hosted-process scenarios, future game patches), the corresponding offset would be read as indeterminate memory and added to every resolved address, silently producing wild detour targets. Default the three fields to 0 at declaration so they at least hold a defined value, and have LoadSections track which of the three sections it actually found. If any are missing, bail out via the existing SHOW_MESSAGE_BOX_AND_EXIT_FILE_LINE path so the failure is visible at startup instead of corrupting hook addresses later. Fixes wopss#134 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
|
...why would CDPR start using obfuscator/whatever else would change default MSVC section names? Game is DRM-free |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #134.
What this changes
Two small changes inside the
Addressesmachinery so the section-offset bug stops being silent:m_codeOffset,m_dataOffset, andm_rdataOffsetto0inAddresses.hpp. They had no in-class initializer and were never set in the constructor, so on a module that did not have.text,.data, and.rdatathey were read as indeterminate memory.LoadSections()track which of the three sections it actually found, and bail out via the existingSHOW_MESSAGE_BOX_AND_EXIT_FILE_LINEpath if any are missing. That turns a silent address-corruption bug into a visible startup failure with file/line context.Why
Every detour target address goes through these offsets in
Addresses.cpp:108-119. With the original code, a missing section meant:DetourAttachpatches arbitrary memory → first hooked call dies in another module, or worse, silently corrupts unrelated code.Cyberpunk's shipping
.exehas all three sections, so this is latent for the normal install today. The reachable paths are thePluginSystem::Load(m_paths.GetExe(), false)self-probe (for any host executable that is not the normal game) and any future patch or third-party tool that renames or merges sections (packers, EAC/DRM wrappers, VMProtect, etc.).What I tested
Read-only review and a local rebuild. I did not try to construct a synthetic exe missing one of the sections; the fix is mechanical enough that I am comfortable with the change without a runtime repro. Happy to add one if you would prefer.
Notes
TerminateProcessis already whatSHOW_MESSAGE_BOX_AND_EXIT_FILE_LINEdoes, so the trailingreturn;is unreachable but harmless and matches the rest ofLoadSections().