Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Feed by tag #264
Feed by tag #264
Changes from 29 commits
c2d2f3d
a5d5881
0bb481c
223343b
d2d31d7
89eb58a
7601bc6
ee59d9e
c60d01e
f74c32f
e622a6f
8db0621
6a7bd53
5f7d27a
665cfba
67d817d
f975bb5
3eb88ac
05ee178
76bbf81
d36c93a
3d8e51e
9216296
93c69a8
9cd44b4
281b49f
fde5155
36f4ea8
2b21c5d
27f8932
d982285
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What happens if I pass in a malicious path here via the site's config? (e.g.,
../../../../../some-secret-file
)?file_exists?
below could reveal the file's existence?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It gets passed to the same "make_page" function as generating the main feed uses. Honestly I'm not sure what happens with malicious paths get used, I assumed that function already included sanity checking for the path. The only change I made to that function was to allow it to include the option to pass in the tag you want a feed for.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@benbalter In reality the
@base
and@path
values never get used:@base
which equals to the value of__dir__
is not used in our use-case because the:read_yaml
is overridden:jekyll-feed/lib/jekyll-feed/page-without-a-file.rb
Lines 5 to 7 in 9cda8c4
That said, your observation cannot be left disregarded (also present in the
master
and published versions) since the instance variable could still be read via another third-party plugin when used outside the GitHub Pages environment.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added a test for this in 93c69a8