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

Add tarfile member sanitization to extractall() #1805

Merged
merged 2 commits into from
Jan 11, 2023
Merged

Conversation

jmgrady
Copy link
Collaborator

@jmgrady jmgrady commented Jan 9, 2023

Create PR to implement CVE-2007-4559 Patch.

The TrellixVulnTeam created a PR to patch our Python scripts to workaround a vulnerability in the tarfile module. From the PR generated by TrellixVulnTeam:

Hi, we are security researchers from the Advanced Research Center at Trellix. We have began a campaign to patch a widespread bug named GHSA-gw9q-c7gh-j9vm. GHSA-gw9q-c7gh-j9vm is a 15 year old bug in the Python tarfile package. By using extract() or extractall() on a tarfile object without sanitizing input, a maliciously crafted .tar file could perform a directory path traversal attack. We found at least one unsantized extractall() in your codebase and are providing a patch for you via pull request. The patch essentially checks to see if all tarfile members will be extracted safely and throws an exception otherwise. We encourage you to use this patch or your own solution to secure against GHSA-gw9q-c7gh-j9vm. Further technical information about the vulnerability can be found in this blog.

If you have further questions you may contact us through this projects lead researcher Kasimir Schulz.

This PR replaces the PR created by Trellix so that we can:

  1. format the Python code so that it passes our CI tests
  2. be able to manage CI actions, such as re-run failed tests
  3. use a branch other than master on their fork of The Combine - since the fork they are trying to merge is called master the CI actions try to deploy the changes to the QA server.

This change is Reviewable

@codecov-commenter
Copy link

Codecov Report

Base: 51.50% // Head: 51.54% // Increases project coverage by +0.03% 🎉

Coverage data is based on head (1d0abbc) compared to base (5700804).
Patch has no changes to coverable lines.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1805      +/-   ##
==========================================
+ Coverage   51.50%   51.54%   +0.03%     
==========================================
  Files         278      278              
  Lines        8595     8595              
  Branches      631      631              
==========================================
+ Hits         4427     4430       +3     
+ Misses       3661     3658       -3     
  Partials      507      507              
Flag Coverage Δ
backend 79.20% <ø> (+0.08%) ⬆️
frontend 32.22% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
Backend/Helper/DuplicateFinder.cs 87.64% <0.00%> (+1.68%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Collaborator

@imnasnainaec imnasnainaec left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @jmgrady)

@jmgrady jmgrady merged commit d199b80 into master Jan 11, 2023
@jmgrady jmgrady deleted the python-tarfile branch January 11, 2023 23:17
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.

3 participants