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

ZipArchive::fromBuffer unconditionally deletes buffer #83

Closed
PazerOP opened this issue Sep 6, 2020 · 2 comments · Fixed by #86
Closed

ZipArchive::fromBuffer unconditionally deletes buffer #83

PazerOP opened this issue Sep 6, 2020 · 2 comments · Fixed by #86

Comments

@PazerOP
Copy link

PazerOP commented Sep 6, 2020

It appears this was bug was introduced in https://github.com/ctabin/libzippp/pull/38/files#diff-b5a89cdf6ac4efa657b832a52b38d47fR102. The libzip documentation for zip_source_buffer_create reads: "If freep is non-zero, the buffer will be freed when it is no longer needed."

As a result, this causes a crash/undefined behavior if it is not valid to delete the buffer when the ZipArchive is deleted.

@PazerOP PazerOP changed the title ZipArchive::openBuffer unconditionally deletes buffer ZipArchive::fromBuffer unconditionally deletes buffer Sep 6, 2020
@ctabin
Copy link
Owner

ctabin commented Sep 7, 2020

Hi @PazerOP and thanks for your report. Could you please post a small example where the described case occurs ?

@PazerOP
Copy link
Author

PazerOP commented Sep 7, 2020

Certainly.

#include <libzippp.h>

int main()
{
	const char* static_buffer = "my zip file, which is in a buffer that should not be deleted";

	auto archive = libzippp::ZipArchive::fromBuffer(static_buffer, strlen(static_buffer));

	delete archive; // Oh no! free(static_buffer) is called
}

This will crash during the delete because free is called on static_buffer. It doesn't matter that the "zip archive" in the buffer is invalid, this is just an example. Imagine that you have a pointer to a zip file in a memory mapped file, or you are managing the lifetime of the buffer yourself.

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 a pull request may close this issue.

2 participants