Merge pull request #343 from sharelatex/bg-block-javascript-property-names

block javascript property names being used as file names

services/web/app/coffee/Features/Project/SafePath.coffee

@@ -23,6 +23,31 @@ load = () ->
 		| (\s+$)    # reject trailing space
 		///g
 
+	# Put a block on filenames which match javascript property names, as they
+	# can cause exceptions where the code puts filenames into a hash. This is a
+	# temporary workaround until the code in other places is made safe against
+	# property names.
+	#
+	# The list of property names is taken from
+	#   ['prototype'].concat(Object.getOwnPropertyNames(Object.prototype))
+	BLOCKEDFILE_RX = ///
+		^(
+			prototype
+			|constructor
+			|toString
+			|toLocaleString
+			|valueOf
+			|hasOwnProperty
+			|isPrototypeOf
+			|propertyIsEnumerable
+			|__defineGetter__
+			|__lookupGetter__
+			|__defineSetter__
+			|__lookupSetter__
+			|__proto__
+		)$
+	///
+
 	MAX_PATH = 1024 # Maximum path length, in characters. This is fairly arbitrary.
 
 	SafePath =
@@ -31,12 +56,15 @@ load = () ->
 			# 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("_")
+			# replace blocked filenames 'prototype' with '@prototype'
+			filename = filename.replace BLOCKEDFILE_RX, "@$1"
 			return filename
 
 		isCleanFilename: (filename) ->
 			return SafePath.isAllowedLength(filename) &&
 				not filename.match(BADCHAR_RX) &&
-				not filename.match(BADFILE_RX)
+				not filename.match(BADFILE_RX) &&
+				not filename.match(BLOCKEDFILE_RX)
 
 		isAllowedLength: (pathname) ->
 			return pathname.length > 0 && pathname.length <= MAX_PATH

services/web/public/coffee/ide/directives/SafePath.coffee

@@ -23,6 +23,31 @@ load = () ->
 		| (\s+$)    # reject trailing space
 		///g
 
+	# Put a block on filenames which match javascript property names, as they
+	# can cause exceptions where the code puts filenames into a hash. This is a
+	# temporary workaround until the code in other places is made safe against
+	# property names.
+	#
+	# The list of property names is taken from
+	#   ['prototype'].concat(Object.getOwnPropertyNames(Object.prototype))
+	BLOCKEDFILE_RX = ///
+		^(
+			prototype
+			|constructor
+			|toString
+			|toLocaleString
+			|valueOf
+			|hasOwnProperty
+			|isPrototypeOf
+			|propertyIsEnumerable
+			|__defineGetter__
+			|__lookupGetter__
+			|__defineSetter__
+			|__lookupSetter__
+			|__proto__
+		)$
+	///
+
 	MAX_PATH = 1024 # Maximum path length, in characters. This is fairly arbitrary.
 
 	SafePath =
@@ -31,12 +56,15 @@ load = () ->
 			# 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("_")
+			# replace blocked filenames 'prototype' with '@prototype'
+			filename = filename.replace BLOCKEDFILE_RX, "@$1"
 			return filename
 
 		isCleanFilename: (filename) ->
 			return SafePath.isAllowedLength(filename) &&
 				not filename.match(BADCHAR_RX) &&
-				not filename.match(BADFILE_RX)
+				not filename.match(BADFILE_RX) &&
+				not filename.match(BLOCKEDFILE_RX)
 
 		isAllowedLength: (pathname) ->
 			return pathname.length > 0 && pathname.length <= MAX_PATH

services/web/test/unit/coffee/Project/SafePathTests.coffee

@@ -63,7 +63,17 @@ describe 'SafePath', ->
 			result = @SafePath.isCleanFilename 'foo\uD800\uDFFFbar'
 			result.should.equal false
 
+		it 'should not accept javascript property names', ->
+			result = @SafePath.isCleanFilename 'prototype'
+			result.should.equal false
 
+		it 'should not accept javascript property names in the prototype', ->
+			result = @SafePath.isCleanFilename 'hasOwnProperty'
+			result.should.equal false
+
+		it 'should not accept javascript property names resulting from substitutions', ->
+			result = @SafePath.isCleanFilename '  proto  '
+			result.should.equal false
 
 		# it 'should not accept a trailing .', ->
 		# 	result = @SafePath.isCleanFilename 'hello.'
@@ -119,5 +129,12 @@ describe 'SafePath', ->
 
 		it 'should replace a multiple leading spaces with ___', ->
 			result = @SafePath.clean '  foo'
-			result.should.equal '__foo'		
+			result.should.equal '__foo'
 
+		it 'should prefix javascript property names with @', ->
+			result = @SafePath.clean 'prototype'
+			result.should.equal '@prototype'
+		
+		it 'should prefix javascript property names in the prototype with @', ->
+			result = @SafePath.clean 'hasOwnProperty'
+			result.should.equal '@hasOwnProperty'