check stat.isFile and isDirectory rather then symlink

services/web/app/coffee/Features/FileStore/FileStoreHandler.coffee

@@ -11,11 +11,12 @@ module.exports = FileStoreHandler =
 	uploadFileFromDisk: (project_id, file_id, fsPath, callback)->
 		fs.lstat fsPath, (err, stat)->
 			if err?
-				logger.err err:err, "error with path symlink check"
-				return callback(err)
-			if stat.isSymbolicLink()
-				logger.log project_id:project_id, file_id:file_id, fsPath:fsPath, "error uploading file from disk, file path is symlink"
-				return callback('file is from symlink')
+				logger.err err:err, project_id:project_id, file_id:file_id, fsPath:fsPath, "error stating file"
+				callback(err)
+			if !stat.isFile()
+				logger.log project_id:project_id, file_id:file_id, fsPath:fsPath, "tried to upload symlink, not contining"
+				return callback(new Error("can not upload symlink"))
+
 			logger.log project_id:project_id, file_id:file_id, fsPath:fsPath, "uploading file from disk"
 			readStream = fs.createReadStream(fsPath)
 			opts =

services/web/app/coffee/Features/Uploads/FileSystemImportManager.coffee

@@ -8,8 +8,8 @@ logger = require("logger-sharelatex")
 
 module.exports = FileSystemImportManager =
 	addDoc: (user_id, project_id, folder_id, name, path, replace, callback = (error, doc)-> )->
-		FileSystemImportManager._isSymLink path, (err, isSymLink)->
-			if isSymLink
+		FileSystemImportManager._isSafeOnFileSystem path, (err, isSafe)->
+			if !isSafe
 				logger.log user_id:user_id, project_id:project_id, folder_id:folder_id, name:name, path:path, "add doc is from symlink, stopping process"
 				return callback("path is symlink")
 			fs.readFile path, "utf8", (error, content = "") ->
@@ -33,8 +33,8 @@ module.exports = FileSystemImportManager =
 					EditorController.addDocWithoutLock project_id, folder_id, name, lines, "upload", callback
 
 	addFile: (user_id, project_id, folder_id, name, path, replace, callback = (error, file)-> )->
-		FileSystemImportManager._isSymLink path, (err, isSymLink)->
-			if isSymLink
+		FileSystemImportManager._isSafeOnFileSystem path, (err, isSafe)->
+			if !isSafe
 				logger.log user_id:user_id, project_id:project_id, folder_id:folder_id, name:name, path:path, "add file is from symlink, stopping insert"
 				return callback("path is symlink")
 
@@ -55,8 +55,8 @@ module.exports = FileSystemImportManager =
 						EditorController.addFileWithoutLock project_id, folder_id, name, path, "upload", callback
 
 	addFolder: (user_id, project_id, folder_id, name, path, replace, callback = (error)-> ) ->
-		FileSystemImportManager._isSymLink path, (err, isSymLink)->
-			if isSymLink
+		FileSystemImportManager._isSafeOnFileSystem path, (err, isSafe)->
+			if !isSafe
 				logger.log user_id:user_id, project_id:project_id, folder_id:folder_id, path:path, "add folder is from symlink, stopping insert"
 				return callback("path is symlink")
 			EditorController.addFolderWithoutLock project_id, folder_id, name, "upload", (error, new_folder) =>
@@ -66,8 +66,8 @@ module.exports = FileSystemImportManager =
 					callback null, new_folder
 
 	addFolderContents: (user_id, project_id, parent_folder_id, folderPath, replace, callback = (error)-> ) ->
-		FileSystemImportManager._isSymLink folderPath, (err, isSymLink)->
-			if isSymLink
+		FileSystemImportManager._isSafeOnFileSystem folderPath, (err, isSafe)->
+			if !isSafe
 				logger.log user_id:user_id, project_id:project_id, parent_folder_id:parent_folder_id, folderPath:folderPath, "add folder contents is from symlink, stopping insert"
 				return callback("path is symlink")
 			fs.readdir folderPath, (error, entries = []) =>
@@ -83,8 +83,8 @@ module.exports = FileSystemImportManager =
 				async.parallelLimit jobs, 5, callback
 
 	addEntity: (user_id, project_id, folder_id, name, path, replace, callback = (error, entity)-> ) ->
-		FileSystemImportManager._isSymLink path, (err, isSymLink)->
-			if isSymLink
+		FileSystemImportManager._isSafeOnFileSystem path, (err, isSafe)->
+			if !isSafe
 				logger.log user_id:user_id, project_id:project_id, folder_id:folder_id, path:path, "add entry is from symlink, stopping insert"
 				return callback("path is symlink")
 
@@ -101,11 +101,11 @@ module.exports = FileSystemImportManager =
 							FileSystemImportManager.addDoc user_id, project_id, folder_id, name, path, replace, callback
 
 
-	_isSymLink: (path, callback = (err, isSymLink)->)->
+	_isSafeOnFileSystem: (path, callback = (err, isSafe)->)->
 		fs.lstat path, (err, stat)->
 			if err?
 				logger.err err:err, "error with path symlink check"
 				return callback(err)
-			isSymLink = stat.isSymbolicLink()
-			callback(err, isSymLink)
+			isSafe = stat.isFile() or stat.isDirectory()
+			callback(err, isSafe)
 

services/web/test/UnitTests/coffee/FileStore/FileStoreHandlerTests.coffee

@@ -10,7 +10,10 @@ describe "FileStoreHandler", ->
 	beforeEach ->
 		@fs =
 			createReadStream : sinon.stub()
-			lstat: sinon.stub().callsArgWith(1, null, {isSymbolicLink:=>@isSymlink})
+			lstat: sinon.stub().callsArgWith(1, null, {
+				isFile:=> @isSafeOnFileSystem
+				isDirectory:-> return false
+			})
 		@writeStream =
 			my:"writeStream"
 			on: (type, cb)-> 
@@ -32,7 +35,7 @@ describe "FileStoreHandler", ->
 	describe "uploadFileFromDisk", ->
 		beforeEach ->
 			@request.returns(@writeStream)
-			@isSymlink = false
+			@isSafeOnFileSystem = true
 
 		it "should create read stream", (done)->
 			@fs.createReadStream.returns 
@@ -78,7 +81,7 @@ describe "FileStoreHandler", ->
 
 		describe "symlink", ->
 			it "should not read file if it is symlink", (done)->
-				@isSymlink = true
+				@isSafeOnFileSystem = false
 				@handler.uploadFileFromDisk @project_id, @file_id, @fsPath, =>
 					@fs.createReadStream.called.should.equal false
 					done()

services/web/test/UnitTests/coffee/Uploads/FileSystemImportManagerTests.coffee

@@ -27,12 +27,12 @@ describe "FileSystemImportManager", ->
 			@docContent = "one\ntwo\nthree"
 			@docLines = @docContent.split("\n")
 			@fs.readFile = sinon.stub().callsArgWith(2, null, @docContent)
-			@FileSystemImportManager._isSymLink = sinon.stub().callsArgWith(1, null, false)
+			@FileSystemImportManager._isSafeOnFileSystem = sinon.stub().callsArgWith(1, null, true)
 
 
 		describe "when path is symlink", ->
 			beforeEach ->
-				@FileSystemImportManager._isSymLink = sinon.stub().callsArgWith(1, null, true)
+				@FileSystemImportManager._isSafeOnFileSystem = sinon.stub().callsArgWith(1, null, false)
 				@EditorController.addDocWithoutLock = sinon.stub()
 				@FileSystemImportManager.addDoc @user_id, @project_id, @folder_id, @name, @path_on_disk, false, @callback
 
@@ -115,7 +115,7 @@ describe "FileSystemImportManager", ->
 	describe "addFile with replace set to false", ->
 		beforeEach ->
 			@EditorController.addFileWithoutLock = sinon.stub().callsArg(5)
-			@FileSystemImportManager._isSymLink = sinon.stub().callsArgWith(1, null, false)
+			@FileSystemImportManager._isSafeOnFileSystem = sinon.stub().callsArgWith(1, null, true)
 			@FileSystemImportManager.addFile @user_id, @project_id, @folder_id, @name, @path_on_disk, false, @callback
 
 		it "should add the file", ->
@@ -125,7 +125,7 @@ describe "FileSystemImportManager", ->
 	describe "addFile with symlink", ->
 		beforeEach ->
 			@EditorController.addFileWithoutLock = sinon.stub()
-			@FileSystemImportManager._isSymLink = sinon.stub().callsArgWith(1, null, true)
+			@FileSystemImportManager._isSafeOnFileSystem = sinon.stub().callsArgWith(1, null, false)
 			@EditorController.replaceFile = sinon.stub()
 			@FileSystemImportManager.addFile @user_id, @project_id, @folder_id, @name, @path_on_disk, false, @callback
 
@@ -142,7 +142,7 @@ describe "FileSystemImportManager", ->
 						name: "not-the-right-file.tex"
 					}]
 				}
-				@FileSystemImportManager._isSymLink = sinon.stub().callsArgWith(1, null, false)
+				@FileSystemImportManager._isSafeOnFileSystem = sinon.stub().callsArgWith(1, null, true)
 				@ProjectLocator.findElement = sinon.stub().callsArgWith(1, null, @folder)
 				@EditorController.addFileWithoutLock = sinon.stub().callsArg(5)
 				@FileSystemImportManager.addFile @user_id, @project_id, @folder_id, @name, @path_on_disk, true, @callback
@@ -167,7 +167,7 @@ describe "FileSystemImportManager", ->
 						name: "not-the-right-file.tex"
 					}]
 				}
-				@FileSystemImportManager._isSymLink = sinon.stub().callsArgWith(1, null, false)
+				@FileSystemImportManager._isSafeOnFileSystem = sinon.stub().callsArgWith(1, null, true)
 				@ProjectLocator.findElement = sinon.stub().callsArgWith(1, null, @folder)
 				@EditorController.replaceFile = sinon.stub().callsArg(4)
 				@FileSystemImportManager.addFile @user_id, @project_id, @folder_id, @name, @path_on_disk, true, @callback
@@ -190,7 +190,7 @@ describe "FileSystemImportManager", ->
 
 		describe "successfully", ->
 			beforeEach ->
-				@FileSystemImportManager._isSymLink = sinon.stub().callsArgWith(1, null, false)
+				@FileSystemImportManager._isSafeOnFileSystem = sinon.stub().callsArgWith(1, null, true)
 				@FileSystemImportManager.addFolder @user_id, @project_id, @folder_id, @name, @path_on_disk, @replace, @callback
 
 			it "should add a folder to the project", ->
@@ -203,7 +203,7 @@ describe "FileSystemImportManager", ->
 
 		describe "with symlink", ->
 			beforeEach ->
-				@FileSystemImportManager._isSymLink = sinon.stub().callsArgWith(1, null, true)
+				@FileSystemImportManager._isSafeOnFileSystem = sinon.stub().callsArgWith(1, null, false)
 				@FileSystemImportManager.addFolder @user_id, @project_id, @folder_id, @name, @path_on_disk, @replace, @callback
 
 			it "should not add a folder to the project", ->
@@ -218,7 +218,7 @@ describe "FileSystemImportManager", ->
 			@FileSystemImportManager.addEntity = sinon.stub().callsArg(6)
 			@FileTypeManager.shouldIgnore = (path, callback) =>
 				callback null, @ignoredEntries.indexOf(require("path").basename(path)) != -1
-			@FileSystemImportManager._isSymLink = sinon.stub().callsArgWith(1, null, false)
+			@FileSystemImportManager._isSafeOnFileSystem = sinon.stub().callsArgWith(1, null, true)
 			@FileSystemImportManager.addFolderContents @user_id, @project_id, @folder_id, @path_on_disk, @replace, @callback
 
 		it "should call addEntity for each file in the folder which is not ignored", ->
@@ -234,12 +234,12 @@ describe "FileSystemImportManager", ->
 		it "should look in the correct directory", ->
 			@fs.readdir.calledWith(@path_on_disk).should.equal true
 
-	describe "addEntity  dddddddd", ->
+	describe "addEntity", ->
 		describe "with directory", ->
 			beforeEach ->
 				@FileTypeManager.isDirectory = sinon.stub().callsArgWith(1, null, true)
 				@FileSystemImportManager.addFolder = sinon.stub().callsArg(6)
-				@FileSystemImportManager._isSymLink = sinon.stub().callsArgWith(1, null, false)
+				@FileSystemImportManager._isSafeOnFileSystem = sinon.stub().callsArgWith(1, null, true)
 				@FileSystemImportManager.addEntity @user_id, @project_id, @folder_id, @name, @path_on_disk, @replace, @callback
 
 			it "should call addFolder", ->
@@ -250,7 +250,7 @@ describe "FileSystemImportManager", ->
 			beforeEach ->
 				@FileTypeManager.isDirectory = sinon.stub().callsArgWith(1, null, false)
 				@FileTypeManager.isBinary = sinon.stub().callsArgWith(2, null, true)
-				@FileSystemImportManager._isSymLink = sinon.stub().callsArgWith(1, null, false)
+				@FileSystemImportManager._isSafeOnFileSystem = sinon.stub().callsArgWith(1, null, true)
 				@FileSystemImportManager.addFile = sinon.stub().callsArg(6)
 				@FileSystemImportManager.addEntity @user_id, @project_id, @folder_id, @name, @path_on_disk, @replace, @callback
 
@@ -263,7 +263,7 @@ describe "FileSystemImportManager", ->
 				@FileTypeManager.isDirectory = sinon.stub().callsArgWith(1, null, false)
 				@FileTypeManager.isBinary = sinon.stub().callsArgWith(2, null, false)
 				@FileSystemImportManager.addDoc = sinon.stub().callsArg(6)
-				@FileSystemImportManager._isSymLink = sinon.stub().callsArgWith(1, null, false)
+				@FileSystemImportManager._isSafeOnFileSystem = sinon.stub().callsArgWith(1, null, true)
 				@FileSystemImportManager.addEntity @user_id, @project_id, @folder_id, @name, @path_on_disk, @replace, @callback
 
 			it "should call addFile", ->