Fix icpr_cert module error handling with missing HRESULT#21663
Open
zeroSteiner wants to merge 2 commits into
Open
Fix icpr_cert module error handling with missing HRESULT#21663zeroSteiner wants to merge 2 commits into
zeroSteiner wants to merge 2 commits into
Conversation
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.
Description
A crash was privately reported to me where the icpr_cert module was failing due to the
hresultbeingnilbecuase the constant didn't exist. When the reporter shared it with me, it was a win32 error with the win32 facility. The changes in this PR handle future, hypothetical scenarios where thehresultlookup fails so you at least get the hex value and can Google (ChatGPT?) it.There's also a gemfile bump that pulls in changes to the windows_error gem which adds lookups for the win32 error code when the facility is win32. These changes are in rapid7/windows_error#9
Related Issue: N/A this was privately reported to me
Breaking Changes
None
Reviewer Notes
Verification Steps
I wasn't able to reproduce the exact conditions where the
hresultwould be nil but there's not much to the logic here. It's pretty straight forward to see that.find_by_retvalcan returnniland then that accessing the message on it would cause a crash.Test Evidence
Environment
AI Usage Disclosure
I only used AI to check my work and only on the rapid7/windows_error PR.
Pre-Submission Checklist
documentation/modules(new modules only)lib/changes)Hardware and Complex Software Module Guidance
If your module targets specialized hardware (routers, IoT, PLCs, etc.) or complex software (licensed, multi-service, or multi-version), provide a pcap, screen recording, or video showing successful execution.
Email sanitized pcaps/recordings to msfdev@metasploit.com — remove real IPs, credentials, and hostnames before sending. If hardware/software is unavailable, explain in the PR description.
Responsiveness and PR Takeover Policy
We want every contribution to make it into the project. If approximately 2 weeks pass after a review request without a comment or code update from you, the team may take over the PR and complete the work on your behalf.
If this happens, you will remain credited as a co-author on the final commit — your contribution is always recognized.
This policy exists to keep the project moving forward. It is not a reflection on the quality of your work or your involvement. Life happens, and we would rather finish the work together than let a good contribution go stale.