Skip to content

libFuzzer: Fix a git_packfile_stream leak#4431

Merged
pks-t merged 1 commit into
libgit2:masterfrom
lhchavez:fix-stream-leak
Dec 8, 2017
Merged

libFuzzer: Fix a git_packfile_stream leak#4431
pks-t merged 1 commit into
libgit2:masterfrom
lhchavez:fix-stream-leak

Conversation

@lhchavez
Copy link
Copy Markdown
Contributor

@lhchavez lhchavez commented Dec 6, 2017

This change ensures that the git_packfile_stream object in
git_indexer_append() does not leak when the stream has errors.

Found using libFuzzer.

This change ensures that the git_packfile_stream object in
git_indexer_append() does not leak when the stream has errors.

Found using libFuzzer.
@lhchavez
Copy link
Copy Markdown
Contributor Author

lhchavez commented Dec 6, 2017

PTAL

Copy link
Copy Markdown
Member

@pks-t pks-t left a comment

Choose a reason for hiding this comment

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

Looks good to me, thank you! Just waiting a short amount of time whether anybody else chimes in.

Comment thread src/indexer.c
return;

if (idx->have_stream)
git_packfile_stream_free(&idx->stream);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Meh, this function doesn't conform to our usual style. 'free' would usually denote the structure itself bein free'd as well, which would have segfaulted here. But it only calls inflateEnd, so this looks good.

@ethomson
Copy link
Copy Markdown
Member

ethomson commented Dec 6, 2017

Thanks for the great test, too! 😻

@pks-t pks-t merged commit 1bf173c into libgit2:master Dec 8, 2017
@pks-t
Copy link
Copy Markdown
Member

pks-t commented Dec 8, 2017

Thanks again for your PR!

@lhchavez lhchavez deleted the fix-stream-leak branch December 15, 2017 15:02
@pks-t pks-t added the backport label Jan 11, 2018
@pks-t pks-t mentioned this pull request Jan 12, 2018
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.

3 participants