Fix memory leaks and secure reference resolution#44
Open
Alvi16 wants to merge 4 commits intokovzol:masterfrom
Open
Fix memory leaks and secure reference resolution#44Alvi16 wants to merge 4 commits intokovzol:masterfrom
Alvi16 wants to merge 4 commits intokovzol:masterfrom
Conversation
- Fix variable name typo in sen_data_evaluate() (line 337) Changed *ref_val to *ret_val to ensure correct error variable is set - Fix NULL pointer dereference in aio_open() (lines 996-1002) Added safety check for proof->everything->tail before dereferencing Prevents crash when loading proofs with no premises Fixes: kovzol#27
kovzol
reviewed
Mar 26, 2026
Owner
There was a problem hiding this comment.
Thanks for your work on this!
I think, the word "dest" was accidentally changed to "dest/src/config.h.in" which is clearly an issue. Please revise.
Author
There was a problem hiding this comment.
Thank you for reporting this! I've searched but I'm unable to locate the issue. Could you please point me to the exact file or line number where "dest" was incorrectly changed? That would help me fix it quickly.
Owner
There was a problem hiding this comment.
Author
|
@kovzol It's good. I think all the changes have been made. You can take a look. |
Owner
|
Thanks, I'll have a look soon. |
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
2 participants
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.
Proposed Changes
This modification addresses several critical issues in the reference resolution loop:
Crash Prevention (Null Pointer Dereference):
The code now checks if
cur_refisNULLafter thels_nthcall before accessing its members. This prevents a systematic crash if a reference is invalid.Memory Leak Fixes:
Previously, the function could return
NULLprematurely without freeing therefsvector. I have added calls todestroy_str_vec(refs)before every error return path.Improved Error Handling:
Instead of a silent
NULLreturn, the function now sets*ret_valtoVALUE_TYPE_ERRORand returns an explicit, translatable error message via_().Note: Tested with invalid references to confirm stability.