diff --git a/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee b/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee index b67180659e..69a518ba25 100644 --- a/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee +++ b/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee @@ -17,6 +17,7 @@ DocstoreManager = require "../Docstore/DocstoreManager" ProjectGetter = require "./ProjectGetter" CooldownManager = require '../Cooldown/CooldownManager' DocumentUpdaterHandler = require('../../Features/DocumentUpdater/DocumentUpdaterHandler') +SafePath = require './SafePath' module.exports = ProjectEntityHandler = getAllFolders: (project_id, callback) -> @@ -179,6 +180,9 @@ module.exports = ProjectEntityHandler = ProjectEntityHandler._addDocWithProject project, folder_id, docName, docLines, userId, callback _addDocWithProject: (project, folder_id, docName, docLines, userId, callback = (error, doc, folder_id, path) ->)=> + # check if docname is allowed + if not SafePath.isCleanFilename docName + return callback new Errors.InvalidNameError("invalid element name") project_id = project._id logger.log project_id: project_id, folder_id: folder_id, doc_name: docName, "adding doc to project with project" confirmFolder project, folder_id, (folder_id)=> @@ -201,6 +205,9 @@ module.exports = ProjectEntityHandler = callback(null, doc, folder_id, result?.path?.fileSystem) restoreDoc: (project_id, doc_id, name, callback = (error, doc, folder_id) ->) -> + # check if docname is allowed (passed in from client so we check it) + if not SafePath.isCleanFilename name + return callback new Errors.InvalidNameError("invalid element name") # getDoc will return the deleted doc's lines, but we don't actually remove # the deleted doc, just create a new one from its lines. ProjectEntityHandler.getDoc project_id, doc_id, include_deleted: true, (error, lines) -> @@ -208,6 +215,9 @@ module.exports = ProjectEntityHandler = ProjectEntityHandler.addDoc project_id, null, name, lines, callback addFileWithoutUpdatingHistory: (project_id, folder_id, fileName, path, userId, callback = (error, fileRef, folder_id, path, fileStoreUrl) ->)-> + # check if file name is allowed + if not SafePath.isCleanFilename fileName + return callback new Errors.InvalidNameError("invalid element name") ProjectGetter.getProject project_id, {rootFolder:true, name:true}, (err, project) -> if err? logger.err project_id:project_id, err:err, "error getting project for add file" @@ -280,7 +290,8 @@ module.exports = ProjectEntityHandler = if !origonalFileRef? logger.err { project_id, folder_id, originalProject_id, origonalFileRef }, "file trying to copy is null" return callback() - fileRef = new File name : origonalFileRef.name + # convert any invalid characters in original file to '_' + fileRef = new File name : SafePath.clean(origonalFileRef.name) FileStoreHandler.copyFile originalProject_id, origonalFileRef._id, project._id, fileRef._id, (err, fileStoreUrl)-> if err? logger.err { err, project_id, folder_id, originalProject_id, origonalFileRef }, "error coping file in s3" @@ -349,6 +360,9 @@ module.exports = ProjectEntityHandler = ProjectEntityHandler.addFolderWithProject project, parentFolder_id, folderName, callback addFolderWithProject: (project, parentFolder_id, folderName, callback = (err, folder, parentFolder_id)->) -> + # check if folder name is allowed + if not SafePath.isCleanFilename folderName + return callback new Errors.InvalidNameError("invalid element name") confirmFolder project, parentFolder_id, (parentFolder_id)=> folder = new Folder name: folderName logger.log project: project._id, parentFolder_id:parentFolder_id, folderName:folderName, "adding new folder" @@ -450,6 +464,9 @@ module.exports = ProjectEntityHandler = renameEntity: (project_id, entity_id, entityType, newName, userId, callback)-> + # check if name is allowed + if not SafePath.isCleanFilename newName + return callback new Errors.InvalidNameError("invalid element name") logger.log(entity_id: entity_id, project_id: project_id, ('renaming '+entityType)) if !entityType? logger.err err: "No entityType set", project_id: project_id, entity_id: entity_id @@ -593,8 +610,10 @@ module.exports = ProjectEntityHandler = return callback(e) type = sanitizeTypeOfElement type - if path.resolve("/", element.name) isnt "/#{element.name}" or element.name.match("/") - e = new Error("invalid element name") + # original check path.resolve("/", element.name) isnt "/#{element.name}" or element.name.match("/") + # check if name is allowed + if not SafePath.isCleanFilename element.name + e = new Errors.InvalidNameError("invalid element name") logger.err project_id:project._id, folder_id:folder_id, element:element, type:type, "failed trying to insert element as name was invalid" return callback(e) @@ -627,7 +646,11 @@ module.exports = ProjectEntityHandler = return callback(err) callback(err, {path:newPath}, project) + checkValidElementName: (folder, name, callback = (err) ->) -> + # check if the path would be too long + if not SafePath.isAllowedLength "#{folder}/#{name}" + return callback new Error.InvalidNameError("path too long") # check if the name is already taken by a doc, file or # folder. If so, return an error "file already exists". err = new Errors.InvalidNameError("file already exists") diff --git a/services/web/app/coffee/Features/Project/SafePath.coffee b/services/web/app/coffee/Features/Project/SafePath.coffee new file mode 100644 index 0000000000..f5b49cdff7 --- /dev/null +++ b/services/web/app/coffee/Features/Project/SafePath.coffee @@ -0,0 +1,47 @@ +# This file is shared between the frontend and server code of web, so that +# filename validation is the same in both implementations. +# Both copies must be kept in sync: +# app/coffee/Features/Project/SafePath.coffee +# public/coffee/ide/directives/SafePath.coffee + +load = () -> + BADCHAR_RX = /// + [ + \/ # no slashes + \* # no asterisk + \u0000-\u001F # no control characters (0-31) + \u007F # no delete + \u0080-\u009F # no unicode control characters (C1) + \uD800-\uDFFF # no unicode surrogate characters + ] + ///g + + BADFILE_RX = /// + (^\.$) # reject . as a filename + | (^\.\.$) # reject .. as a filename + | (^\s+) # reject leading space + | (\s+$) # reject trailing space + ///g + + MAX_PATH = 1024 # Maximum path length, in characters. This is fairly arbitrary. + + SafePath = + clean: (filename) -> + filename = filename.replace BADCHAR_RX, '_' + # for BADFILE_RX replace any matches with an equal number of underscores + filename = filename.replace BADFILE_RX, (match) -> + return new Array(match.length + 1).join("_") + return filename + + isCleanFilename: (filename) -> + return SafePath.isAllowedLength(filename) && + not filename.match(BADCHAR_RX) && + not filename.match(BADFILE_RX) + + isAllowedLength: (pathname) -> + return pathname.length > 0 && pathname.length <= MAX_PATH + +if define? + define [], load +else + module.exports = load() \ No newline at end of file diff --git a/services/web/test/unit/coffee/Project/SafePathTests.coffee b/services/web/test/unit/coffee/Project/SafePathTests.coffee new file mode 100644 index 0000000000..50b44b7e2c --- /dev/null +++ b/services/web/test/unit/coffee/Project/SafePathTests.coffee @@ -0,0 +1,123 @@ +chai = require('chai') +assert = require('chai').assert +should = chai.should() +expect = chai.expect +sinon = require 'sinon' +modulePath = "../../../../app/js/Features/Project/SafePath" +SandboxedModule = require('sandboxed-module') + +describe 'SafePath', -> + beforeEach -> + @SafePath = SandboxedModule.require modulePath + + describe 'isCleanFilename', -> + it 'should accept a valid filename "main.tex"', -> + result = @SafePath.isCleanFilename 'main.tex' + result.should.equal true + + it 'should not accept an empty filename', -> + result = @SafePath.isCleanFilename '' + result.should.equal false + + it 'should not accept / anywhere', -> + result = @SafePath.isCleanFilename 'foo/bar' + result.should.equal false + + it 'should not accept .', -> + result = @SafePath.isCleanFilename '.' + result.should.equal false + + it 'should not accept ..', -> + result = @SafePath.isCleanFilename '..' + result.should.equal false + + it 'should not accept * anywhere', -> + result = @SafePath.isCleanFilename 'foo*bar' + result.should.equal false + + it 'should not accept leading whitespace', -> + result = @SafePath.isCleanFilename ' foobar.tex' + result.should.equal false + + it 'should not accept trailing whitespace', -> + result = @SafePath.isCleanFilename 'foobar.tex ' + result.should.equal false + + it 'should not accept leading and trailing whitespace', -> + result = @SafePath.isCleanFilename ' foobar.tex ' + result.should.equal false + + it 'should not accept control characters (0-31)', -> + result = @SafePath.isCleanFilename 'foo\u0010bar' + result.should.equal false + + it 'should not accept control characters (127, delete)', -> + result = @SafePath.isCleanFilename 'foo\u007fbar' + result.should.equal false + + it 'should not accept control characters (128-159)', -> + result = @SafePath.isCleanFilename 'foo\u0080\u0090bar' + result.should.equal false + + it 'should not accept surrogate characters (128-159)', -> + result = @SafePath.isCleanFilename 'foo\uD800\uDFFFbar' + result.should.equal false + + + + # it 'should not accept a trailing .', -> + # result = @SafePath.isCleanFilename 'hello.' + # result.should.equal false + + + # it 'should not accept \\', -> + # result = @SafePath.isCleanFilename 'foo\\bar' + # result.should.equal false + + describe 'isAllowedLength', -> + it 'should accept a valid path "main.tex"', -> + result = @SafePath.isAllowedLength 'main.tex' + result.should.equal true + + it 'should not accept an extremely long path', -> + longPath = new Array(1000).join("/subdir") + '/main.tex' + result = @SafePath.isAllowedLength longPath + result.should.equal false + + it 'should not accept an empty path', -> + result = @SafePath.isAllowedLength '' + result.should.equal false + + describe 'clean', -> + it 'should not modify a valid filename', -> + result = @SafePath.clean 'main.tex' + result.should.equal 'main.tex' + + it 'should replace invalid characters with _', -> + result = @SafePath.clean 'foo/bar*/main.tex' + result.should.equal 'foo_bar__main.tex' + + it 'should replace "." with "_"', -> + result = @SafePath.clean '.' + result.should.equal '_' + + it 'should replace ".." with "__"', -> + result = @SafePath.clean '..' + result.should.equal '__' + + it 'should replace a single trailing space with _', -> + result = @SafePath.clean 'foo ' + result.should.equal 'foo_' + + it 'should replace a multiple trailing spaces with ___', -> + result = @SafePath.clean 'foo ' + result.should.equal 'foo__' + + it 'should replace a single leading space with _', -> + result = @SafePath.clean ' foo' + result.should.equal '_foo' + + it 'should replace a multiple leading spaces with ___', -> + result = @SafePath.clean ' foo' + result.should.equal '__foo' +