[microTVM] Mitigate security vulnerability CVE-2007-4559 #13751
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Collaborator
|
Thanks for contributing to TVM! Please refer to the contributing guidelines https://tvm.apache.org/docs/contribute/ for useful information and tips. Please request code reviews from Reviewers by @-ing them in a comment. Generated by tvm-bot |
gromero
requested changes
Jan 10, 2023
Contributor
gromero
left a comment
gromero
left a comment
There was a problem hiding this comment.
@YuchenJin Hi. Thanks a lot for the patch! I've left some comments inline, but otherwise LGTM.
The CI errors are trivial to fix, just remove the period after the PR title and commit title when sending the PR again.
gromero
changed the title
Contributor
|
@YuchenJin I've tweaked a bit the PR description to tell give more context on how this CVE could affect the microTVM specifically. |
Contributor
|
cc @guberti since it also touches Arduino and also because tvm-bot didn't tag him :-) |
Contributor
|
@YuchenJin Friendly ping :-) |
Contributor
Author
Contributor
|
@tvm-bot rerun |
Contributor
|
Failed to re-run CI in https://github.com/apache/tvm/actions/runs/4055383350 Detailswith response |
Contributor
|
@driazati Hi. Do you know why I can't retrigger the CI in this PR? (see error above). |
Member
|
Not sure exactly what went wrong with Jenkins but it seems like there are a few legit CI errors still remaining |
Contributor
Member
|
The lint error (https://ci.tlcpack.ai/blue/organizations/jenkins/tvm-lint/detail/PR-13751/3/pipeline) is definitely real so this will at least need a fix for that, a rebase probably wouldn't hurt either |
Contributor
@driazati ah, fair enough! I was looking at a different log: https://ci.tlcpack.ai/blue/rest/organizations/jenkins/pipelines/tvm-arm/branches/PR-13751/runs/3/nodes/104/steps/142/log/?start=0 |
gromero
requested changes
Jan 31, 2023
Contributor
gromero
left a comment
gromero
left a comment
There was a problem hiding this comment.
@YuchenJin Thanks a lot for addressing the comments. Please see my new comments inline.
Also, since the exception message got longer, it's necessary to break the line accordingly to satisfy the linter (see David's comment above). Also, could you please rebase (not merge) your code onto HEAD? Thanks!
| def _safe_extract(tar, path=".", members=None, *, numeric_owner=False): | ||
| def is_within_directory(directory, member): | ||
|
|
||
| target = os.path.join(path, member.name) |
Contributor
There was a problem hiding this comment.
path here is being taken from the outer scope, i.e. from path="." arg in _safe_extract. path is also passed to _is_within_directory() as directory arg, so could it be changed to actually:
target = os.path.join(directory, member.name)
?
nit: If you wish, this line could be put immediately before the line where target is used, i.e. before line 185 (abs_target = ...).
Contributor
Author
There was a problem hiding this comment.
Thank you @gromero for your suggestions! I rebased and updated, let's see if the ci complains this time. :)
gromero
changed the title
Contributor
|
@tvm-bot rerun |
Contributor
|
Failed to re-run CI in https://github.com/apache/tvm/actions/runs/4078496473 Detailswith response |
Member
|
@tvm-bot rerun |
|
Hi, just a random heads up that this fix for path traversal in |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
7 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Borrowed from tlc-pack/relax#335. The original author is @TrellixVulnTeam from the Advanced Research Center at Trellix.
This PR patches the security vulnerability CVE-2007-4559 in the Arduino and Zephyr Project API servers. CVE-2007-4559 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 Model Library Format file could perform a directory path traversal attack, silently overwriting files outside the target project target dir when creating new projects, for instance. The patch essentially checks to see if all tarfile members will be extracted safely and throws an exception otherwise. Further technical information about the vulnerability can be found in this blog.
Co-authored-by: TrellixVulnTeam kasimir.schulz@trellix.com