Skip to content

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

Merged
jekyllbot merged 2 commits intomasterfrom
entryfilter-symlink-fix
Sep 7, 2018
Merged

Security: fix include bypass of EntryFilter#filter symlink check#7226
jekyllbot merged 2 commits intomasterfrom
entryfilter-symlink-fix

Conversation

@parkr
Copy link
Copy Markdown
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
Copy Markdown
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.
@parkr parkr force-pushed the entryfilter-symlink-fix branch from 3dec37d to d0dbd5a Compare September 7, 2018 17:47
Copy link
Copy Markdown
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
Copy Markdown
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.

4 participants