From 8a10e98b5686cd03d37b4b5b32a240890876d56d Mon Sep 17 00:00:00 2001
From: Brian Gough <bjg@network-theory.co.uk>
Date: Fri, 16 Feb 2018 10:08:04 +0000
Subject: [PATCH] block javascript property names being used as file names

---
 .../coffee/Features/Project/SafePath.coffee   | 30 ++++++++++++++++++-
 .../coffee/ide/directives/SafePath.coffee     | 30 ++++++++++++++++++-
 .../unit/coffee/Project/SafePathTests.coffee  | 19 +++++++++++-
 3 files changed, 76 insertions(+), 3 deletions(-)

diff --git a/services/web/app/coffee/Features/Project/SafePath.coffee b/services/web/app/coffee/Features/Project/SafePath.coffee
index f5b49cdff7..fff60a3724 100644
--- a/services/web/app/coffee/Features/Project/SafePath.coffee
+++ b/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
diff --git a/services/web/public/coffee/ide/directives/SafePath.coffee b/services/web/public/coffee/ide/directives/SafePath.coffee
index f5b49cdff7..fff60a3724 100644
--- a/services/web/public/coffee/ide/directives/SafePath.coffee
+++ b/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
diff --git a/services/web/test/unit/coffee/Project/SafePathTests.coffee b/services/web/test/unit/coffee/Project/SafePathTests.coffee
index 50b44b7e2c..82f645997c 100644
--- a/services/web/test/unit/coffee/Project/SafePathTests.coffee
+++ b/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'