Patch show-stopping security vulnerabilities #1944

Merged
merged 14 commits into from Jan 14, 2014

Projects

None yet

5 participants

@parkr
Member
parkr commented Jan 14, 2014

Two vulnerabilities found:

  1. Post#destination allows path traversal due to the CGI.unescape called prior to the post URL being used in the generation of the output file path. URL escaped characters can be used in a permalink to bypass the filtering provided by URL#sanitize_url. (@gregose)
  2. Arbitrary file reads via symlinks: it's possible to read anywhere on the filesystem by placing a symlink to a directory in _includes. (@charliesome)

GitHub Pages has already been patched. It is strongly recommended that any other Jekyll hosts upgrade to v1.4.3 when it lands (tonight).

benbalter and others added some commits Jan 7, 2014
@benbalter @parkr benbalter failing test
Signed-off-by: Parker Moore <parkrmoore@gmail.com>
823f875
@benbalter @parkr benbalter url escape before sanitizing
Signed-off-by: Parker Moore <parkrmoore@gmail.com>
a06dff4
@benbalter @parkr benbalter fix failing post count test
Signed-off-by: Parker Moore <parkrmoore@gmail.com>
827eed7
@benbalter @parkr benbalter test multiple traversals
Signed-off-by: Parker Moore <parkrmoore@gmail.com>
93a6d4d
@benbalter @parkr benbalter add symlink failing test
Signed-off-by: Parker Moore <parkrmoore@gmail.com>
0339710
@benbalter @parkr benbalter unbreak tests
Signed-off-by: Parker Moore <parkrmoore@gmail.com>
69b4399
@benbalter @parkr benbalter fix symlink so tests fail
Signed-off-by: Parker Moore <parkrmoore@gmail.com>
e7cf42b
@benbalter @parkr benbalter rebreak tests, move sanitization closer to write
Signed-off-by: Parker Moore <parkrmoore@gmail.com>
6277335
@benbalter @parkr benbalter test symlinkd dir, not file
Signed-off-by: Parker Moore <parkrmoore@gmail.com>
b1a7e14
@benbalter @parkr benbalter patch symlink vuln and properly test
Signed-off-by: Parker Moore <parkrmoore@gmail.com>
98b366e
@benbalter @parkr benbalter escape relative post permalinks, cleanup
Signed-off-by: Parker Moore <parkrmoore@gmail.com>
10d1f49
@alindeman @parkr alindeman Prevents disclosure of file existence
Signed-off-by: Parker Moore <parkrmoore@gmail.com>
b55fb3d
@benbalter @parkr benbalter sanity check for pages permalink traversal
Signed-off-by: Parker Moore <parkrmoore@gmail.com>
3901c88
@mattr- mattr- was assigned Jan 14, 2014
@parkr parkr merged commit 71bb028 into v1-stable Jan 14, 2014

1 check failed

default The Travis CI build failed
Details
@parkr parkr added a commit that referenced this pull request Jan 14, 2014
@parkr parkr Update history to reflect merge of #1944 5325903
@parkr parkr deleted the vuln-patch branch Jan 14, 2014
@shiyj

Have you test this on windows?

D:\appstore\rubyapp\shiyj-jekyll>irb
irb(main):001:0> url="/tags/aaa.html"
=> "/tags/aaa.html"
irb(main):002:0> File.expand_path(url,"/")
=> "D:/tags/aaa.html"

the result of expand_path is #{disk drive number} + #{url} on ruby1.93,windows.
so I'll get a path like "d:/jekyll/_site/d:/tags/aaa.html" after the destination method,
while the "d:/jekyll/_site/tags/aaa.html" is what i want.

Member

Yep, we're aware of this problem (#1948). Basically what we're trying to do is this:

  1. Get the inputted URL's absolute path (as though it were in a browser, relative to the base FQDN)
  2. Append it to the destination directory

Goal: Don't allow any URL's to be outside the destination directory.

This fix works for unix systems where / is the root of the filesystem, but not for Windows. How would you suggest fixing it? Remove (\w+:)\/?

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