additional validation of requests

services/clsi/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"

services/clsi/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 ->