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

clam-1650 fixing leak when exporting pdf json metadata #553

Merged
merged 1 commit into from
May 31, 2022

Conversation

m-sola
Copy link
Contributor

@m-sola m-sola commented Apr 19, 2022

Cleanup for the pdf->stats structure wasn't guaranteed when handling
a fatal memory error. This fix pulls stats cleanup into its own function
and handles the case where there's a fatal memory error by moving the
function call below the err: tag.

This fix also removes an extraneous call to pdf_export_json.

@ragusaa
Copy link
Contributor

ragusaa commented May 9, 2022

There is a block of code in cli_pdf with an '#if 0 ... #endif' around it. Could you remove that as part of the PR?

@ragusaa
Copy link
Contributor

ragusaa commented May 10, 2022

Everything else looks good to me. I reproduced the issue in main, and verified that it is gone with your changes, so I'll go ahead and approve it.

@ragusaa
Copy link
Contributor

ragusaa commented May 10, 2022

Could you also add a merge request to the private fuzz corpus?

Cleanup for the pdf->stats structure wasn't guaranteed when handling
a fatal memory error. This fix pulls stats cleanup into its own function
and handles the case where there's a fatal memory error by moving the
function call below the err: tag.

This fix also removes an extraneous call to pdf_export_json.
@micahsnyder
Copy link
Contributor

Rebased to see it go through the test pipelines with the PoC in there.

@micahsnyder micahsnyder merged commit 6d5d295 into Cisco-Talos:main May 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants