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

[release-5.29] Backport #2403 aka CVE-2024-3727 #2415

Closed
wants to merge 6 commits into from

Conversation

dcermak
Copy link
Contributor

@dcermak dcermak commented May 14, 2024

No description provided.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
(cherry picked from commit b724ee7)
Use defer() to remove the temporary file, instead
of duplicating the call.

Should not change behavior.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
(cherry picked from commit a802d65)
Use defer, a nested function, and early returns.

Besides being a bit more directly related to what
we want to achieve, this now does not call decompressed.Close()
on a nil value if DecompressStream fails.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
(cherry picked from commit 4a3785d)
... to prevent unexpected behavior on invalid values.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
(cherry picked from commit a9225e4)
If doing it makes sense at all, it should happen before
the values are used.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
(cherry picked from commit 2bcb834)
... to prevent panics if the value does not contain a :, or other unexpected
values (e.g. a path traversal).

Don't bother on paths where we computed the digest ourselves, or it is already trusted
for other reasons.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
(cherry picked from commit 39e7c91)
@mtrmac
Copy link
Collaborator

mtrmac commented May 15, 2024

Thanks so much for the backport!

If you don’t mind, I’d prefer the variant in #2418 , where the commit order matches #2404 , making it easier to prove the two match; and the backport is ~1 line smaller.

Either way, I’m very grateful for this PR, because it independently confirms the correctness of the backport.

Alternatively, if you’d like to reorder/update this PR, I’d be happy to merge this one to preserve your credit in the git history.

@mtrmac mtrmac changed the title Backport #2403 aka CVE-2024-3727 [release-5.29] Backport #2403 aka CVE-2024-3727 May 15, 2024
@dcermak
Copy link
Contributor Author

dcermak commented May 15, 2024

Alternatively, if you’d like to reorder/update this PR, I’d be happy to merge this one to preserve your credit in the git history.

Nah, don't worry about it, you did 99% of the work, I just deleted some lines in git conflicts ;-)

@dcermak dcermak closed this May 15, 2024
@dcermak dcermak deleted the backport-2403 branch May 15, 2024 15:23
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.

None yet

2 participants