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

Correct upload dir path validation logic #1338

Merged
merged 3 commits into from
Nov 11, 2023
Merged

Conversation

rdmark
Copy link
Member

@rdmark rdmark commented Nov 11, 2023

The safe path validation for file uploads with dropzone.js wasn't actually working following a recent refactor meant to reduce code duplication and cognitive complexity. The error response was happening in the wrong flask context and the file transfer was allowed when it should have been stopped.

Additionally, the file downloads validation, which used the same refactored method, ended up using the wrong type of flask response, leading to the app being aborted with a 403 instead of a flash warning displayed in the flask app.

Therefore, this patch removes the refactored method and brings the is_safe_path() check and response into the respective flask endpoints.

Two peripheral fixes pushed while working on this:

  • tweaked the labels of the file download form, to make it clear the purpose of the target dir dropdown.
  • clean up existing tmp file before uploading (otherwise the file size validation fails in the end.)

@rdmark rdmark requested a review from akuker as a code owner November 11, 2023 03:27
@rdmark rdmark marked this pull request as draft November 11, 2023 03:27
@rdmark rdmark marked this pull request as ready for review November 11, 2023 05:45
Copy link

sonarcloud bot commented Nov 11, 2023

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

0.0% 0.0% Coverage
6.9% 6.9% Duplication

idea Catch issues before they fail your Quality Gate with our IDE extension sonarlint SonarLint

@rdmark rdmark merged commit 8d26807 into develop Nov 11, 2023
9 of 12 checks passed
@rdmark rdmark deleted the rdmark-path-validation-bug branch November 11, 2023 11:46
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