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

Fix code scanning alert no. 61: Arbitrary file access during archive extraction ("Zip Slip") #1223

Merged
merged 1 commit into from
Jan 10, 2025

Conversation

lfoppiano
Copy link
Collaborator

Fixes https://github.com/kermitt2/grobid/security/code-scanning/61

To fix the problem, we need to ensure that the output paths constructed from zip archive entries are validated to prevent writing files to unexpected locations. This can be done by verifying that the normalized full path of the output file starts with a prefix that matches the destination directory. We will use java.nio.file.Path for path normalization and prefix checking.

  1. Normalize the path of the file to be created.
  2. Check if the normalized path starts with the destination directory path.
  3. If the check fails, throw an exception to prevent writing the file.

Suggested fixes powered by Copilot Autofix. Review carefully before merging.

…Zip Slip")

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
@coveralls
Copy link

Coverage Status

coverage: 40.829% (-0.006%) from 40.835%
when pulling 2cc090f on alert-autofix-61
into 7c0bccf on master.

@lfoppiano lfoppiano marked this pull request as ready for review January 10, 2025 08:47
@lfoppiano
Copy link
Collaborator Author

This is not a real issue because the problematic code is from the main class. No impact whatsoever.

@lfoppiano lfoppiano merged commit 2ab61a6 into master Jan 10, 2025
7 of 10 checks passed
@lfoppiano lfoppiano deleted the alert-autofix-61 branch January 10, 2025 08:47
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