[MIRROR] [s] Security vulnerability patch [MDB IGNORE] (#9256)

* [s] Security vulnerability patch (#62568)

About The Pull Request

In my personal, subjective opinion; trialmins should not, in fact, be able to read and delete server/box configuration files on a whim.

cl
server: Patches multiple(?) arbitrary file related vulnerabilities
/cl

* [s] Security vulnerability patch

Co-authored-by: TheFakeElon <59686430+TheFakeElon@users.noreply.github.com>

code/__DEFINES/text.dm

@@ -17,3 +17,6 @@
 
 ///Adds a html style to a text string. Hacky, but that's how inputted text appear on paper sheets after going through the UI.
 #define PAPER_MARK_TEXT(text, color, font) "<span style=\"color:[color];font-family:'[font]';\">[text]</span>\n \n"
+
+/// Folder directory for strings
+#define STRING_DIRECTORY "strings"

code/__HELPERS/_string_lists.dm

@@ -7,8 +7,9 @@ GLOBAL_LIST(string_cache)
 GLOBAL_VAR(string_filename_current_key)
 
 
-/proc/strings_replacement(filename, key, directory = "strings")
-	load_strings_file(filename, directory)
+/proc/strings_replacement(filename, key)
+	filename = SANITIZE_FILENAME(filename)
+	load_strings_file(filename)
 
 	if((filename in GLOB.string_cache) && (key in GLOB.string_cache[filename]))
 		var/response = pick(GLOB.string_cache[filename][key])
@@ -16,19 +17,21 @@ GLOBAL_VAR(string_filename_current_key)
 		response = r.Replace(response, /proc/strings_subkey_lookup)
 		return response
 	else
-		CRASH("strings list not found: [directory]/[filename], index=[key]")
+		CRASH("strings list not found: [STRING_DIRECTORY]/[filename], index=[key]")
 
-/proc/strings(filename as text, key as text, directory = "strings")
-	load_strings_file(filename, directory)
+/proc/strings(filename as text, key as text)
+	filename = SANITIZE_FILENAME(filename)
+	load_strings_file(filename)
 	if((filename in GLOB.string_cache) && (key in GLOB.string_cache[filename]))
 		return GLOB.string_cache[filename][key]
 	else
-		CRASH("strings list not found: [directory]/[filename], index=[key]")
+		CRASH("strings list not found: [STRING_DIRECTORY]/[filename], index=[key]")
 
 /proc/strings_subkey_lookup(match, group1)
 	return pick_list(GLOB.string_filename_current_key, group1)
 
-/proc/load_strings_file(filename, directory = "strings")
+/proc/load_strings_file(filename)
+	filename = SANITIZE_FILENAME(filename) // in case we're called directly
 	GLOB.string_filename_current_key = filename
 	if(filename in GLOB.string_cache)
 		return //no work to do
@@ -36,7 +39,7 @@ GLOBAL_VAR(string_filename_current_key)
 	if(!GLOB.string_cache)
 		GLOB.string_cache = new
 
-	if(fexists("[directory]/[filename]"))
-		GLOB.string_cache[filename] = json_load("[directory]/[filename]")
+	if(fexists("[STRING_DIRECTORY]/[filename]"))
+		GLOB.string_cache[filename] = json_load("[STRING_DIRECTORY]/[filename]")
 	else
-		CRASH("file not found: [directory]/[filename]")
+		CRASH("file not found: [STRING_DIRECTORY]/[filename]")

code/__HELPERS/files.dm

@@ -1,4 +1,3 @@
-
 /**
  * For FTP requests. (i.e. downloading runtime logs.)
  *

code/_globalvars/regexes.dm

@@ -10,9 +10,9 @@ GLOBAL_DATUM_INIT(is_color, /regex, regex("^#\[0-9a-fA-F]{6}$"))
 //finds text strings recognized as links on discord. Mainly used to stop embedding.
 GLOBAL_DATUM_INIT(has_discord_embeddable_links, /regex, regex("(https?://\[^\\s|<\]{2,})"))
 
-//All < and > characters 
+//All < and > characters
 GLOBAL_DATUM_INIT(angular_brackets, /regex, regex(@"[<>]", "g"))
 
-//All characters forbidden by filenames: ", \, \n, \t, /, ?, %, *, :, |, <, >
-GLOBAL_DATUM_INIT(filename_forbidden_chars, /regex, regex(@{""|[\\\n\t/?%*:|<>]"}, "g"))
+//All characters forbidden by filenames: ", \, \n, \t, /, ?, %, *, :, |, <, >, ..
+GLOBAL_DATUM_INIT(filename_forbidden_chars, /regex, regex(@{""|[\\\n\t/?%*:|<>]|\.\."}, "g"))
 // had to use the OR operator for quotes instead of putting them in the character class because it breaks the syntax highlighting otherwise.

code/controllers/configuration/configuration.dm

@@ -32,7 +32,7 @@
 
 	/// A regex that matches words blocked IC, but not in PDAs
 	var/static/regex/ic_outside_pda_filter_regex
-	
+
 	/// A regex that matches words soft blocked IC
 	var/static/regex/soft_ic_filter_regex
 
@@ -347,7 +347,7 @@ Example config:
 
 		switch (command)
 			if ("map")
-				currentmap = load_map_config("_maps/[data].json")
+				currentmap = load_map_config(data)
 				if(currentmap.defaulted)
 					log_config("Failed to load map config for [data]!")
 					currentmap = null

code/controllers/subsystem/mapping.dm

@@ -454,9 +454,9 @@ GLOBAL_LIST_EMPTY(the_station_areas)
 
 /datum/controller/subsystem/mapping/proc/preloadRuinTemplates()
 	// Still supporting bans by filename
-	var/list/banned = generateMapList("[global.config.directory]/lavaruinblacklist.txt")
-	banned += generateMapList("[global.config.directory]/spaceruinblacklist.txt")
-	banned += generateMapList("[global.config.directory]/iceruinblacklist.txt")
+	var/list/banned = generateMapList("lavaruinblacklist.txt")
+	banned += generateMapList("spaceruinblacklist.txt")
+	banned += generateMapList("iceruinblacklist.txt")
 
 	for(var/item in sort_list(subtypesof(/datum/map_template/ruin), /proc/cmp_ruincost_priority))
 		var/datum/map_template/ruin/ruin_type = item
@@ -491,7 +491,7 @@ GLOBAL_LIST_EMPTY(the_station_areas)
 //SKYRAT EDIT END//
 
 /datum/controller/subsystem/mapping/proc/preloadShuttleTemplates()
-	var/list/unbuyable = generateMapList("[global.config.directory]/unbuyableshuttles.txt")
+	var/list/unbuyable = generateMapList("unbuyableshuttles.txt")
 
 	for(var/item in subtypesof(/datum/map_template/shuttle))
 		var/datum/map_template/shuttle/shuttle_type = item

code/datums/map_config.dm

@@ -34,14 +34,15 @@
 	/// Dictionary of job sub-typepath to template changes dictionary
 	var/job_changes = list()
 
-/proc/load_map_config(filename = "data/next_map.json", default_to_box, delete_after, error_if_missing = TRUE)
+/proc/load_map_config(filename = "next_map.json", default_to_box, delete_after, error_if_missing = TRUE)
+	filename = "_maps/[filename].json"
 	var/datum/map_config/config = new
 	if (default_to_box)
 		return config
 	if (!config.LoadConfig(filename, error_if_missing))
 		qdel(config)
 		config = new /datum/map_config  // Fall back to Box
-	if (delete_after)
+	else if (delete_after)
 		fdel(filename)
 	return config
 

code/game/machinery/camera/camera.dm

@@ -328,10 +328,9 @@
 			itemname = pressed_pda.name
 			info = pressed_pda.notehtml
 
-		var/sanitized_name = sanitize(itemname)
-
+		itemname = sanitize(itemname)
 		to_chat(paper_user, span_notice("You hold \the [itemname] up to the camera..."))
-		paper_user.log_talk(sanitized_name, LOG_GAME, log_globally=TRUE, tag="Pressed to camera")
+		paper_user.log_talk(itemname, LOG_GAME, log_globally=TRUE, tag="Pressed to camera")
 		paper_user.changeNext_move(CLICK_CD_MELEE)
 
 		for(var/mob/potential_viewer in GLOB.player_list)
@@ -343,11 +342,11 @@
 					to_chat(AI, "[span_name(paper_user)] holds <a href='?_src_=usr;show_paper=1;'>\a [itemname]</a> up to one of your cameras ...")
 				else
 					to_chat(AI, "<b><a href='?src=[REF(AI)];track=[html_encode(paper_user.name)]'>[paper_user]</a></b> holds <a href='?_src_=usr;show_paper=1;'>\a [itemname]</a> up to one of your cameras ...")
-				AI.log_talk(sanitized_name, LOG_VICTIM, tag="Pressed to camera from [key_name(paper_user)]", log_globally=FALSE)
+				AI.log_talk(itemname, LOG_VICTIM, tag="Pressed to camera from [key_name(paper_user)]", log_globally=FALSE)
 				AI.last_paper_seen = "<HTML><HEAD><TITLE>[itemname]</TITLE></HEAD><BODY><TT>[info]</TT></BODY></HTML>"
 			else if (potential_viewer.client.eye == src)
 				to_chat(potential_viewer, "[span_name("[paper_user]")] holds \a [itemname] up to one of the cameras ...")
-				potential_viewer.log_talk(sanitized_name, LOG_VICTIM, tag="Pressed to camera from [key_name(paper_user)]", log_globally=FALSE)
+				potential_viewer.log_talk(itemname, LOG_VICTIM, tag="Pressed to camera from [key_name(paper_user)]", log_globally=FALSE)
 				potential_viewer << browse(text("<HTML><HEAD><TITLE>[]</TITLE></HEAD><BODY><TT>[]</TT></BODY></HTML>", itemname, info), text("window=[]", itemname))
 		return
 

code/modules/awaymissions/zlevel.dm

@@ -43,6 +43,7 @@ GLOBAL_LIST_INIT(potentialConfigRandomZlevels, generateConfigMapList(directory =
 
 /proc/generateMapList(filename)
 	. = list()
+	filename = "[global.config.directory]/[SANITIZE_FILENAME(filename)]"
 	var/list/Lines = world.file2list(filename)
 
 	if(!Lines.len)

code/modules/tgs/v3210/api.dm

@@ -54,6 +54,8 @@
 	return ""
 
 /datum/tgs_api/v3210/proc/file2list(filename)
+	if(IsAdminAdvancedProcCall())
+		CRASH("Attempted to read file via admin call")
 	return splittext(trim_left(trim_right(file2text(filename))), "\n")
 
 /datum/tgs_api/v3210/OnWorldNew(minimum_required_security_level)

code/modules/tgui/tgui_window.dm

@@ -321,3 +321,6 @@
 			// Resend the assets
 			for(var/asset in sent_assets)
 				send_asset(asset)
+
+/datum/tgui_window/vv_edit_var(var_name, var_value)
+	return var_name != NAMEOF(src, id) && ..()