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 1651 #2: Fix leak loading malformed PDB database #532

Merged
merged 3 commits into from
May 27, 2022

Conversation

ragusaa
Copy link
Contributor

@ragusaa ragusaa commented Apr 6, 2022

This addresses the pdb memory leaks in the ticket. Will do separate PRs for other issues that are not in the same files.

Fixes: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=43849
Fixes: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=44115

Copy link
Contributor

@val-ms val-ms left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like this was changed (slightly differently) in https://github.com/Cisco-Talos/clamav/pull/530/files#diff-c2d7ebcb1295f163964c14d878838c83e69061189c318f01909d34dd5a383a98L427-L466

This version:

  • isn't clang-formatted
  • doesn't use the VERIFY_POINTER macros
  • does have better error handling for the parse_regex() call.

How do you want to proceed? Do you want to merge these two PR's? Do you want to take the improvements from this one and put them in #530? Or?

@val-ms
Copy link
Contributor

val-ms commented Apr 26, 2022

From offline conversation: because of the overlap with #530, the plan is to fix-up that one and merge that first -- then rebase this and fix the merge conflicts, and then merge this. So until #530 lands, this one is blocked.

@ragusaa ragusaa force-pushed the CLAM-1651-MemoryLeaks branch 2 times, most recently from fb4b395 to 06d1c39 Compare May 17, 2022 18:24
@ragusaa
Copy link
Contributor Author

ragusaa commented May 17, 2022

The updates to PR-530 appear to have removed the merge conflicts, because when I did a 'git pull', there was nothing to merge. I clang-formatted, so I am ready for re-review.

@ragusaa ragusaa force-pushed the CLAM-1651-MemoryLeaks branch from 06d1c39 to b9519c1 Compare May 17, 2022 21:39
@ragusaa ragusaa force-pushed the CLAM-1651-MemoryLeaks branch from 1efecfd to 3b91926 Compare May 17, 2022 22:59
@val-ms
Copy link
Contributor

val-ms commented May 27, 2022

Everything checks out. This was a little difficult to verify because the 2 PoC's for this were mixed in with 4 PoC's for related issues. But manual testing confirmed all is well with this PR.

@val-ms val-ms merged commit 62bf3be into Cisco-Talos:main May 27, 2022
@val-ms val-ms changed the title Clam 1651 memory leaks Clam 1651 regex_suffix memory leaks May 28, 2022
@val-ms val-ms changed the title Clam 1651 regex_suffix memory leaks Clam 1651 PDB load, regex_suffix memory leaks May 28, 2022
@val-ms val-ms changed the title Clam 1651 PDB load, regex_suffix memory leaks Clam 1651 #2: Fix leak loading malformed PDB database, regex_suffix memory leaks May 28, 2022
@val-ms val-ms changed the title Clam 1651 #2: Fix leak loading malformed PDB database, regex_suffix memory leaks Clam 1651 #2: Fix leak loading malformed PDB database May 28, 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.

2 participants