Skip to content
This repository has been archived by the owner on Sep 6, 2021. It is now read-only.

Fix #13099: Disallow user to create files/folder using relative path #13256

Merged
merged 7 commits into from Feb 20, 2018
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 33 additions & 27 deletions src/project/ProjectModel.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,40 +70,47 @@ define(function (require, exports, module) {

/**
* @private
* A string containing all invalid characters for a specific platform.
* This will be used to construct a regular expression for checking invalid filenames.
* When a filename with one of these invalid characters are detected, then it is
* also used to substitute the place holder of the error message.
* RegEx to validate a file path.
*/
var _invalidChars;
var _invalidChars = /([?\*\|\<\>"]+|\/{2,}|\.{2,}|\.$)/i;

/**
* @private
* RegEx to validate if a filename is not allowed even if the system allows it.
* This is done to prevent cross-platform issues.
*/

var _illegalFilenamesRegEx = /^(\.+|com[1-9]|lpt[1-9]|nul|con|prn|aux|)$|\.+$/i;
var _illegalFilenamesRegEx = /((\b(com[0-9]+|lpt[0-9]+|nul|con|prn|aux)\b)|\.+$|\/+|\\+|\:)/i;

/**
* Returns true if this matches valid filename specifications.
* See http://msdn.microsoft.com/en-us/library/windows/desktop/aa365247(v=vs.85).aspx
*
* TODO: This likely belongs in FileUtils.
*
* @param {string} filename to check
* @param {string} invalidChars List of characters that are disallowed
* @return {boolean} true if the filename is valid
*/
function isValidFilename(filename, invalidChars) {
// Validate file name
// Checks for valid Windows filenames:
// See http://msdn.microsoft.com/en-us/library/windows/desktop/aa365247(v=vs.85).aspx
function isValidFilename(filename) {
// Fix issue adobe#13099
// See https://github.com/adobe/brackets/issues/13099
return !(
new RegExp("[" + invalidChars + "]+").test(filename) ||
_illegalFilenamesRegEx.test(filename)
filename.match(_invalidChars)|| filename.match(_illegalFilenamesRegEx)
//filename.match(_invalidChars) || filename.match(_illegalFilenamesRegEx)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This got left in by accident, please remove.

);
}

/**
* Returns true if given path is valid.
*
* @param {string} path to check
* @return {boolean} true if the filename is valid
*/
function isValidPath(path) {
// Fix issue adobe#13099
// See https://github.com/adobe/brackets/issues/13099
return !(path.match(_invalidChars));
}

/**
* @private
* @see #shouldShow
Expand Down Expand Up @@ -181,9 +188,16 @@ define(function (require, exports, module) {
*/
function doCreate(path, isFolder) {
var d = new $.Deferred();
var filename = FileUtils.getBaseName(path);

// Check if filename
if (!isValidFilename(filename)){
return d.reject(ERROR_INVALID_FILENAME).promise();
}

var name = FileUtils.getBaseName(path);
if (!isValidFilename(name, _invalidChars)) {
// Check if fullpath with filename is valid
// This check is used to circumvent directory jumps (Like ../..)
if (!isValidPath(path)) {
return d.reject(ERROR_INVALID_FILENAME).promise();
}

Expand Down Expand Up @@ -906,7 +920,7 @@ define(function (require, exports, module) {

if (oldPath === newPath) {
result.resolve();
} else if (!isValidFilename(newName, _invalidChars)) {
} else if (!isValidFilename(newName)) {
result.reject(ERROR_INVALID_FILENAME);
} else {
var entry = isFolder ? FileSystem.getDirectoryForPath(oldPath) : FileSystem.getFileForPath(oldPath);
Expand Down Expand Up @@ -1345,25 +1359,17 @@ define(function (require, exports, module) {
return welcomeProjects.indexOf(pathNoSlash) !== -1;
}

// Init invalid characters string
if (brackets.platform === "mac") {
_invalidChars = "?*|:/";
} else if (brackets.platform === "linux") {
_invalidChars = "?*|/";
} else {
_invalidChars = "/?*:<>\\|\""; // invalid characters on Windows
}

exports._getWelcomeProjectPath = _getWelcomeProjectPath;
exports._addWelcomeProjectPath = _addWelcomeProjectPath;
exports._isWelcomeProjectPath = _isWelcomeProjectPath;
exports._ensureTrailingSlash = _ensureTrailingSlash;
exports._shouldShowName = _shouldShowName;
exports._invalidChars = _invalidChars;
exports._invalidChars = "? * | : / < > \\ | \" ..";

exports.shouldShow = shouldShow;
exports.defaultIgnoreGlobs = defaultIgnoreGlobs;
exports.isValidFilename = isValidFilename;
exports.isValidPath = isValidPath;
exports.EVENT_CHANGE = EVENT_CHANGE;
exports.EVENT_SHOULD_SELECT = EVENT_SHOULD_SELECT;
exports.EVENT_SHOULD_FOCUS = EVENT_SHOULD_FOCUS;
Expand Down
106 changes: 100 additions & 6 deletions test/spec/ProjectModel-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -311,17 +311,111 @@ define(function (require, exports, module) {
});
});

describe("isValidPath", function () {
it("returns true for UNIX style file path", function () {
expect(ProjectModel.isValidPath("/tmp/src/test/")).toBe(true);
});

it("returns true for WINDOWS style file path", function () {
expect(ProjectModel.isValidPath("C:\\tmp\\src\\test\\")).toBe(true);
});

it("returns false for path that contains an invalid char \'..\'", function () {
expect(ProjectModel.isValidPath("../tmp/src/test/")).toBe(false);
});

it("returns false for path that contains an invalid char \'../..\'", function () {
expect(ProjectModel.isValidPath("../../tmp/src/test/")).toBe(false);
});

it("returns false for path that contains an invalid char \'..\\..\'", function () {
expect(ProjectModel.isValidPath("..\\..\\tmp\\src\\test\\")).toBe(false);
});
});

describe("isValidFilename", function () {
it("returns true for filenames with nothing invalid", function () {
expect(ProjectModel.isValidFilename("foo.txt", "*")).toBe(true);
it("returns true for simple filename", function () {
expect(ProjectModel.isValidFilename("foo.txt")).toBe(true);
});

it("returns true for filename that starts with a '\.\'", function () {
expect(ProjectModel.isValidFilename(".tmp")).toBe(true);
});

it("returns false for filenames that has ends with a \'.\'", function () {
expect(ProjectModel.isValidFilename("dummy.")).toBe(false);
});

it("returns false for filename that contains an invalid char \'?\'", function () {
expect(ProjectModel.isValidFilename("foo?txt")).toBe(false);
});

it("returns false for filename that contains an invalid char \'*\'", function () {
expect(ProjectModel.isValidFilename("foo\*txt")).toBe(false);
});

it("returns false for filename that contains an invalid char \'\|\'", function () {
expect(ProjectModel.isValidFilename("foo\|txt")).toBe(false);
});

it("returns false for filename that contains an invalid char \'\:\'", function () {
expect(ProjectModel.isValidFilename("foo\:txt")).toBe(false);
});

it("returns false for filename that contains an invalid char \'\<\'", function () {
expect(ProjectModel.isValidFilename("foo\<txt")).toBe(false);
});

it("returns false for filename that contains an invalid char \'\>\'", function () {
expect(ProjectModel.isValidFilename("foo\>txt")).toBe(false);
});

it("returns false for filename that contains an invalid char \'/\'", function () {
expect(ProjectModel.isValidFilename("directory/foo.txt")).toBe(false);
});

it("returns false for filename that contains an invalid char \'//\'", function () {
expect(ProjectModel.isValidFilename("directory//foo.txt")).toBe(false);
});

it("returns false for filenames that match the invalid characters", function () {
expect(ProjectModel.isValidFilename("foo*txt", "|*")).toBe(false);
it("returns false for filename that contains an invalid char \'\\\'", function () {
expect(ProjectModel.isValidFilename("directory\\foo.txt")).toBe(false);
});

it("returns false for filename that contains an invalid char \'\\\\\'", function () {
expect(ProjectModel.isValidFilename("directory\\\\foo.txt")).toBe(false);
});

it("returns false for filename that contains an invalid char \'..\'", function () {
expect(ProjectModel.isValidFilename("..foo")).toBe(false);
});

it("returns false for filename that contains an invalid char \'..\\..\'", function () {
expect(ProjectModel.isValidFilename("..\\foo")).toBe(false);
});

it("returns false for filenames that has invalid name \'com1\'", function () {
expect(ProjectModel.isValidFilename("com1")).toBe(false);
});

it("returns false for filenames that has invalid name \'lpt\'", function () {
expect(ProjectModel.isValidFilename("lpt1")).toBe(false);
});

it("returns false for filenames that has invalid name \'nul\'", function () {
expect(ProjectModel.isValidFilename("nul")).toBe(false);
});

it("returns false for filenames that has invalid name \'con\'", function () {
expect(ProjectModel.isValidFilename("con")).toBe(false);
});

it("returns false for filenames that has invalid name \'prn\'", function () {
expect(ProjectModel.isValidFilename("prn")).toBe(false);
});

it("returns false for filenames that match the internal list of disallowed names", function () {
expect(ProjectModel.isValidFilename("/test/prn")).toBe(false);
it("returns false for filenames that has invalid name \'aux\'", function () {
expect(ProjectModel.isValidFilename("aux")).toBe(false);
});

});
Expand Down