From 9d3fdcf8b4046d7f7b45409f7dc404569610a286 Mon Sep 17 00:00:00 2001
From: Brian Gough <bjg@network-theory.co.uk>
Date: Mon, 11 May 2015 12:10:13 +0100
Subject: [PATCH] additional validation of requests

---
 app/coffee/StaticServerForbidSymlinks.coffee  | 19 +++++-
 .../StaticServerForbidSymlinksTests.coffee    | 65 +++++++++++++++++++
 2 files changed, 83 insertions(+), 1 deletion(-)

diff --git a/app/coffee/StaticServerForbidSymlinks.coffee b/app/coffee/StaticServerForbidSymlinks.coffee
index 2f48190..348ecca 100644
--- a/app/coffee/StaticServerForbidSymlinks.coffee
+++ b/app/coffee/StaticServerForbidSymlinks.coffee
@@ -9,7 +9,24 @@ module.exports = ForbidSymlinks = (staticFn, root, options) ->
 	basePath = Path.resolve(root)
 	return (req, res, next) ->
 		path = url.parse(req.url)?.pathname
-		requestedFsPath = Path.normalize("#{basePath}/#{path}")
+		# check that the path is of the form /project_id/path/to/file
+		if result = path.match(/^\/?(\w+)\/(.*)/)
+			project_id = result[1]
+			file = result[2]
+		else
+			logger.warn path: path, "unrecognized file request"
+			return res.sendStatus(404)
+		# check that the file does not use a relative path
+		for dir in file.split('/')
+			if dir == '..'
+				logger.warn path: path, "attempt to use a relative path"
+				return res.sendStatus(404)
+		# check that the requested path is normalized
+		requestedFsPath = "#{basePath}/#{project_id}/#{file}"
+		if requestedFsPath != Path.normalize(requestedFsPath)
+				logger.error path: requestedFsPath, "requestedFsPath is not normalized"
+				return res.sendStatus(404)
+		# check that the requested path is not a symlink
 		fs.realpath requestedFsPath, (err, realFsPath)->
 			if err?
 				logger.warn err:err, requestedFsPath:requestedFsPath, realFsPath:realFsPath, path: req.params[0], project_id: req.params.project_id, "error checking file access"
diff --git a/test/unit/coffee/StaticServerForbidSymlinksTests.coffee b/test/unit/coffee/StaticServerForbidSymlinksTests.coffee
index e17bace..e6b7f5f 100644
--- a/test/unit/coffee/StaticServerForbidSymlinksTests.coffee
+++ b/test/unit/coffee/StaticServerForbidSymlinksTests.coffee
@@ -20,6 +20,7 @@ describe "StaticServerForbidSymlinks", ->
 			"logger-sharelatex": 
 				log:->
 				warn:->
+				error:->
 			"fs":@fs
 
 		@dummyStatic = (rootDir, options) ->
@@ -69,6 +70,70 @@ describe "StaticServerForbidSymlinks", ->
 				done()
 			@StaticServerForbidSymlinks @req, @res
 
+
+	describe "with a relative file", ->
+		beforeEach ->
+			@req.url = "/12345/../67890/output.pdf"
+
+		it "should send a 404", (done)->
+			@res.sendStatus = (resCode)->
+				resCode.should.equal 404
+				done()
+			@StaticServerForbidSymlinks @req, @res
+
+
+	describe "with a unnormalized file containing .", ->
+		beforeEach ->
+			@req.url = "/12345/foo/./output.pdf"
+
+		it "should send a 404", (done)->
+			@res.sendStatus = (resCode)->
+				resCode.should.equal 404
+				done()
+			@StaticServerForbidSymlinks @req, @res
+
+
+	describe "with a file containing an empty path", ->
+		beforeEach ->
+			@req.url = "/12345/foo//output.pdf"
+
+		it "should send a 404", (done)->
+			@res.sendStatus = (resCode)->
+				resCode.should.equal 404
+				done()
+			@StaticServerForbidSymlinks @req, @res
+
+	describe "with a non-project file", ->
+		beforeEach ->
+			@req.url = "/.foo/output.pdf"
+
+		it "should send a 404", (done)->
+			@res.sendStatus = (resCode)->
+				resCode.should.equal 404
+				done()
+			@StaticServerForbidSymlinks @req, @res
+
+	describe "with a file outside the compiledir", ->
+		beforeEach ->
+			@req.url = "/../bar/output.pdf"
+
+		it "should send a 404", (done)->
+			@res.sendStatus = (resCode)->
+				resCode.should.equal 404
+				done()
+			@StaticServerForbidSymlinks @req, @res
+
+
+	describe "with a file with no leading /", ->
+		beforeEach ->
+			@req.url = "./../bar/output.pdf"
+
+		it "should send a 404", (done)->
+			@res.sendStatus = (resCode)->
+				resCode.should.equal 404
+				done()
+			@StaticServerForbidSymlinks @req, @res
+
 	describe "with an error from fs.realpath", ->
 
 		beforeEach ->