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 support for extension user files #3433

Merged
merged 1 commit into from
Feb 26, 2021

Conversation

aledeg
Copy link
Member

@aledeg aledeg commented Feb 9, 2021

Closes #1709 (possibly)

Changes proposed in this pull request:

  • Add support for extension user files

How to test the feature manually:

Pull request checklist:

  • clear commit messages
  • code manually tested
  • unit tests written (optional if too hard)
  • documentation updated

Additional information can be found in the documentation.

Extension user files can be stored easily in the user folder instead of the
static folder.

Extension user files can be stored easily in the user folder instead of the
static folder.
@aledeg
Copy link
Member Author

aledeg commented Feb 9, 2021

Please let me know if something is missing.

@Alkarex Alkarex added this to the 1.18.0 milestone Feb 10, 2021
@@ -48,7 +53,12 @@ function is_valid_path_extension($path, $extensionPath) {
return false;
}

// File to serve must be under a `ext_dir/static/` directory.
// User files do not need further validations
if (!$isStatic) {
Copy link
Member

Choose a reason for hiding this comment

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

We should not allow arbitrary paths in the filesystem, so $path needs to be constrained somehow, at least preventing things such as ../

Copy link
Member Author

Choose a reason for hiding this comment

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

This validation is skipped only for files generated by the extension configuration in the user folder. And the validation specifies that it must be in the user folder.
I am confused why we should be testing static for those files. Because in the event of the use of ../, it has to be contained in the user folder. So I guess the risks are really low.

@Alkarex Alkarex merged commit 0ce798d into FreshRSS:master Feb 26, 2021
@aledeg aledeg deleted the enhance/extension-configuration branch February 26, 2021 18:19
Alkarex added a commit to Alkarex/FreshRSS that referenced this pull request Dec 8, 2022
Details later.
Due to FreshRSS#3433 (1.18.0)
@Alkarex Alkarex mentioned this pull request Dec 8, 2022
Alkarex added a commit that referenced this pull request Dec 8, 2022
Details later.
Due to #3433 (1.18.0)
@Alkarex
Copy link
Member

Alkarex commented Dec 8, 2022

Security regression #4928
@aledeg We need to move this functionality to a normal controller to check that we are serving a file from the proper user.
ext.php should be limited to files not requiring login.

@Alkarex
Copy link
Member

Alkarex commented Dec 8, 2022

#4930

Alkarex added a commit that referenced this pull request Dec 8, 2022
Details later.
Due to #3433 (1.18.0)
@Alkarex
Copy link
Member

Alkarex commented Apr 6, 2024

Taking advantage of this PR: #6267

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Minz_Extensions and accessing files
2 participants