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

Security: fix include bypass of EntryFilter#filter symlink check #7226

Merged
merged 2 commits into from Sep 7, 2018

Conversation

parkr
Copy link
Member

@parkr parkr commented Sep 7, 2018

In EntryFilter#filter, we check for include? before we check for symlink?, which allows symlinks to be read in a build when they shouldn't be by just including them.

/cc #7224

@parkr parkr requested a review from a team September 7, 2018 17:32
@parkr parkr added the security label Sep 7, 2018
@ashmaroli ashmaroli changed the title master: EntryFilter#filter symlink fix EntryFilter: Reject all symlinks, even if explicitly included Sep 7, 2018
@parkr parkr changed the title EntryFilter: Reject all symlinks, even if explicitly included Security: fix include bypass of EntryFilter#filter symlink check Sep 7, 2018
@DirtyF
Copy link
Member

DirtyF commented Sep 7, 2018

👍 once we appease Rubocop

Previously, you could include the name of a symlinked file
and Jekyll would not filter it. This is considered a bypass
of the symlink checking, and thus a security bug.
Copy link
Member

@ashmaroli ashmaroli left a comment

Choose a reason for hiding this comment

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

👍 LGTM!

@parkr
Copy link
Member Author

parkr commented Sep 7, 2018

@jekyllbot: merge +bug

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

Successfully merging this pull request may close these issues.

None yet

4 participants