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 optimize_thumbnail raises Detected path traversal attempt (#633) #634

Merged
merged 1 commit into from
Jul 26, 2024

Conversation

bmihelac
Copy link
Contributor

No description provided.

@benkonrath
Copy link

I hit this issue and I can confirm that this PR is working well.

@MHM5000
Copy link

MHM5000 commented Jul 23, 2024

Is there any chance this could go to master today? and a new version could be released?

@jrief jrief merged commit 5b9b115 into SmileyChris:master Jul 26, 2024
4 checks passed
@PetrDlouhy
Copy link
Contributor

PetrDlouhy commented Jul 26, 2024

Hm. Now I see that this is not fix for all problems caused by the fixes of CVE-2024-39330.
Tests here were run before the testing matrix update to Django 4.2+.
Tests still do fail on django.core.exceptions.SuspiciousFileOperation, so I expect there are other places where path is used instead of name, I will try to investigate.

@jrief The tests are also not passing because of this:

The KeyError: 0 is caused by commit fe51af8 which changes number of version parameters.

@bmihelac
Copy link
Contributor Author

@PetrDlouhy I'll check this too. When I was looking at the issue I installed the package it as editable and tested against website with latest Django 4.2.+

@PetrDlouhy
Copy link
Contributor

@bmihelac I didn't find a solution so far. The problem is with ImageClearableFileInput, so you might not have it in the testing project. I am still not sure if it is just problem with tests or if it is problem also on real projects.

@PetrDlouhy
Copy link
Contributor

@bmihelac I am also wondering. If this didn't fix the tests is the the fix covered? Maybe we should add a test that would fail if the fix is not present.

@bmihelac
Copy link
Contributor Author

bmihelac commented Jul 29, 2024

I am also wondering. If this didn't fix the tests is the the fix covered? Maybe we should add a test that would fail if the fix is not present.

Tests exists but unfortunately they were not run with Django 4.2.14 as you noted in #634 (comment)

The second issue is that specific tests were not run in the local environment if the testfixtures package was not installed. I added a pull request regarding this issue:

#640

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.

5 participants