Potential fix for code scanning alert no. 11: Access of invalid pointer#104
Merged
Conversation
Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
| unsafe { | ||
| // At this point we have asserted that voltage_metadata_ptr is non-null, | ||
| // so it is safe to create a reference and use it below. | ||
| let voltage_metadata = &*voltage_metadata_ptr; |
Check failure
Code scanning / CodeQL
Access of invalid pointer High
Copilot Autofix
AI about 2 months ago
Copilot could not generate an autofix suggestion
Copilot could not generate an autofix suggestion for this alert. Try pushing a new commit or if the problem persists contact support.
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.
Potential fix for https://github.com/MWATelescope/mwalib/security/code-scanning/11
In general, to fix this kind of problem you must ensure that any pointer obtained via FFI is validated in the same scope where it is dereferenced, and that code paths where the pointer might still be null or invalid are excluded before dereference (e.g., by returning early, panicking, or otherwise aborting). Assertions in a different
unsafeblock are not enough for static analysis and are fragile if the FFI function misbehaves.The best targeted fix here, without changing functionality, is:
mwalib_voltage_metadata_getbut treat non‑zero return values as fatal by panicking with the error message. This keeps tests strict and clarifies that subsequent code assumes success.voltage_metadata_ptrfor null again (inside the same function, in safe Rust) and panic if it is null. This reinforces the contract and avoids undefined behavior if the FFI function erroneously returns success with a null pointer.buffer_len, avoid dereferencingvoltage_metadata_ptrdirectly; instead, bind a temporary referencelet voltage_metadata = &*voltage_metadata_ptr;after the null check and use that to access fields. That makes it explicit that we only dereference after validating non-null and keeps the unsafe dereference localized.unsafeblocks aroundmwalib_voltage_metadata_getand the dereference, or at least add the added null check and reference binding in the same region where the dereference occurs.We only need to modify the function that contains the shown snippet in
src/voltage_context/ffi_test.rs. No new methods or imports are required; we can reuseCStringand the standard library. The functional behavior remains the same on success paths; on failure or inconsistent FFI behavior, the test will now panic before dereferencing an invalid pointer.Suggested fixes powered by Copilot Autofix. Review carefully before merging.