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

Add folder verification on delete file/image #2

Closed
wants to merge 1 commit into from

Conversation

NunoLopesPT
Copy link

Update the example to check if the file to be removed is inside the uploads folder to make sure unwanted files are deleted

Discussion:
froala/wysiwyg-editor-php-sdk#14

Documentation Changes Required:
https://www.froala.com/wysiwyg-editor/docs/sdks/php/file-server-delete
https://www.froala.com/wysiwyg-editor/docs/sdks/php/image-server-delete

$upload_folder = "/uploads/";

// If the file is inside the uploads folder
if (substr($src, 0, strlen($upload_folder)) === $upload_folder)
Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't really fix the problem because the path might be something like this: /uploads/../foo/something_bad_here. Also, this should be done in the SDK, not in the examples.

Copy link
Author

Choose a reason for hiding this comment

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

Why the user would have something bad in the uploads folder?
The SDK don't have any kind of "users" extensions to check permissions if the user can delete the file, so how should this be handled?

Copy link
Contributor

Choose a reason for hiding this comment

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

Inside the SDK when deleting a file, there should be a check to see if the $upload_folder actually contains the file indicated by the path. The easiest way would be to look into the $upload_folder files recursively and compare paths. Again, this should be done in the SDK itself, not here.

stefanneculai pushed a commit that referenced this pull request Feb 27, 2019
kapil2704 pushed a commit that referenced this pull request Jun 16, 2020
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