Skip to content

Correct upload dir path validation logic#1338

Merged
rdmark merged 3 commits intodevelopfrom
rdmark-path-validation-bug
Nov 11, 2023
Merged

Correct upload dir path validation logic#1338
rdmark merged 3 commits intodevelopfrom
rdmark-path-validation-bug

Conversation

@rdmark
Copy link
Copy Markdown
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 force-pushed the rdmark-path-validation-bug branch from 50dbfaf to 8208bf5 Compare November 11, 2023 05:32
@rdmark rdmark marked this pull request as ready for review November 11, 2023 05:45
@rdmark rdmark force-pushed the rdmark-path-validation-bug branch from 7a197d6 to f79aee8 Compare November 11, 2023 06:23
@sonarqubecloud
Copy link
Copy Markdown

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
@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