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

prevent possible segfault when creating seek table #4201

Merged
merged 1 commit into from
Nov 27, 2024

Conversation

rorosen
Copy link
Contributor

@rorosen rorosen commented Nov 24, 2024

Add a check in ZSTD_seekTable_create_fromSeekable whether the seek table of a ZSTD_seekable is initialized before creating a new seek table from it. Formerly, the function created a segfault when the seek table of ZSTD_seekablewas not initialized.

Fixes #4200

@@ -252,6 +252,7 @@ size_t ZSTD_seekable_free(ZSTD_seekable* zs)

ZSTD_seekTable* ZSTD_seekTable_create_fromSeekable(const ZSTD_seekable* zs)
{
if (zs->seekTable.entries == NULL) return NULL;
Copy link
Contributor

@Cyan4973 Cyan4973 Nov 25, 2024

Choose a reason for hiding this comment

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

Since we are dereferencing zs here, I would prefer an assert(zs != NULL); just before this statement.

Arguably, it should have been added a few lines below too, but wasn't.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for your review. I added the assert

Copy link
Contributor

@Cyan4973 Cyan4973 left a comment

Choose a reason for hiding this comment

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

Indeed, given that ZSTD_seekTable is essentially a limited extract from ZSTD_seekable, it involves copying some content from one side to the other.
And it's a problem if the source doesn't exist (is not initialized).
This simple check is a good fix.

@Cyan4973 Cyan4973 self-assigned this Nov 25, 2024
Add a check whether the seek table of a `ZSTD_seekable` is initialized
before creating a new seek table from it. Return `NULL`, if the check
fails.
@rorosen rorosen force-pushed the seek-table-create-null-check branch from affa711 to b683c0d Compare November 25, 2024 07:57
@Cyan4973 Cyan4973 merged commit 82d4705 into facebook:dev Nov 27, 2024
94 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ZSTD_seekTable_create_fromSeekable can cause segmentation faults
3 participants