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

[Doc Repo] Fix path traversal security issue #3678

Merged
merged 2 commits into from
May 28, 2018
Merged

[Doc Repo] Fix path traversal security issue #3678

merged 2 commits into from
May 28, 2018

Conversation

johnsaigle
Copy link
Contributor

@johnsaigle johnsaigle commented May 17, 2018

See Redmine #14511.

Related to #3603, which contains a better fix + more significant refactoring.

This PR provides the minimal changes necessary to fix the vulnerability.

PapillonMcGill
PapillonMcGill previously approved these changes May 22, 2018
Copy link
Contributor

@PapillonMcGill PapillonMcGill left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not tested but eliminate use of forged data.
Suggest a "Security" label for security fix

@johnsaigle
Copy link
Contributor Author

johnsaigle commented May 22, 2018

@PapillonMcGill I agree that a Security label should be created. I'll put it in the LORIS meeting agenda.

@@ -78,6 +78,7 @@


$target_path = $base_path . $fileBase;
error_log("Target path is: $target_path");
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left over debug statement.. this shouldn't be in the error log.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done!

@driusan driusan merged commit 206131c into aces:bugfix May 28, 2018
@ridz1208 ridz1208 added this to the 19.1.1 milestone May 28, 2018
@johnsaigle johnsaigle added the Category: Security PR or issue that aims to improve security label Jul 23, 2018
@johnsaigle johnsaigle deleted the 180517-PathTraversalFix branch August 7, 2018 19:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Category: Bug PR or issue that aims to report or fix a bug Category: Security PR or issue that aims to improve security
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants