diff --git a/services/web/app/coffee/Features/FileStore/FileStoreHandler.coffee b/services/web/app/coffee/Features/FileStore/FileStoreHandler.coffee index 20af15676f..09790cf99f 100644 --- a/services/web/app/coffee/Features/FileStore/FileStoreHandler.coffee +++ b/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 = diff --git a/services/web/app/coffee/Features/Uploads/FileSystemImportManager.coffee b/services/web/app/coffee/Features/Uploads/FileSystemImportManager.coffee index a8b9f639fe..04c7c3bcc7 100644 --- a/services/web/app/coffee/Features/Uploads/FileSystemImportManager.coffee +++ b/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) diff --git a/services/web/test/UnitTests/coffee/FileStore/FileStoreHandlerTests.coffee b/services/web/test/UnitTests/coffee/FileStore/FileStoreHandlerTests.coffee index c35eefcf87..8f9d8a555e 100644 --- a/services/web/test/UnitTests/coffee/FileStore/FileStoreHandlerTests.coffee +++ b/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() diff --git a/services/web/test/UnitTests/coffee/Uploads/FileSystemImportManagerTests.coffee b/services/web/test/UnitTests/coffee/Uploads/FileSystemImportManagerTests.coffee index 4fbea218bb..33013be91b 100644 --- a/services/web/test/UnitTests/coffee/Uploads/FileSystemImportManagerTests.coffee +++ b/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", ->