[PORTS] A bunch of sanitization fixes (#5380)

## About The Pull Request Includes a bunch of monkestation-only sanitization fixes. Also ports following PRs: tgstation/tgstation#89049 tgstation/tgstation#84091 ## Why It's Good For The Game Exploits are [bad](#5340)! (Yes, it's possible to do this with any improperly sanitized input) ## Changelog :cl: leaKsi, FlufflesTheDog, MrMelbert fix: (FlufflesTheDog) Sanitization on citation pda alerts. fix: (MrMelbert) Sanitization for pAIs and medical/security records. fix: Sanitization for mechanical components, pathology records, integrated circuit/shells, and PDA app notifications. /:cl: --------- Co-authored-by: Lucy <[email protected]>
Monkestation · Feb 10, 2025 · 9ed2c47 · 9ed2c47
1 parent 2ca6a89
commit 9ed2c47
Show file tree

Hide file tree

Showing 11 changed files with 36 additions and 28 deletions.
diff --git a/code/controllers/subsystem/pai.dm b/code/controllers/subsystem/pai.dm
@@ -52,18 +52,18 @@ SUBSYSTEM_DEF(pai)
 		return FALSE
 	switch(action)
 		if("submit")
-			candidate.comments = trim(params["comments"], MAX_BROADCAST_LEN)
-			candidate.description = trim(params["description"], MAX_BROADCAST_LEN)
-			candidate.name = trim(params["name"], MAX_NAME_LEN)
+			candidate.comments = reject_bad_name(params["comments"], allow_numbers = TRUE, max_length = MAX_BROADCAST_LEN, strict = TRUE, cap_after_symbols = FALSE) || "Unknown"
+			candidate.description = reject_bad_name(params["description"], allow_numbers = TRUE, max_length = MAX_BROADCAST_LEN, strict = TRUE, cap_after_symbols = FALSE) || "Unknown"
+			candidate.name = reject_bad_name(params["name"], allow_numbers = TRUE, max_length = MAX_NAME_LEN, strict = TRUE, cap_after_symbols = FALSE) || "Unknown"
 			candidate.ckey = user.ckey
 			candidate.ready = TRUE
 			ui.close()
 			submit_alert(user)
 			return TRUE
 		if("save")
-			candidate.comments = params["comments"]
-			candidate.description = params["description"]
-			candidate.name = params["name"]
+			candidate.comments = reject_bad_name(params["comments"], allow_numbers = TRUE, max_length = MAX_BROADCAST_LEN, strict = TRUE, cap_after_symbols = FALSE) || "Unknown"
+			candidate.description = reject_bad_name(params["description"], allow_numbers = TRUE, max_length = MAX_BROADCAST_LEN, strict = TRUE, cap_after_symbols = FALSE) || "Unknown"
+			candidate.name = reject_bad_name(params["name"], allow_numbers = TRUE, max_length = MAX_NAME_LEN, strict = TRUE, cap_after_symbols = FALSE) || "Unknown"
 			candidate.savefile_save(user)
 			return TRUE
 		if("load")

diff --git a/code/game/machinery/computer/records/medical.dm b/code/game/machinery/computer/records/medical.dm
@@ -94,7 +94,9 @@
 		if("add_note")
 			if(!params["content"])
 				return FALSE
-			var/content = trim(params["content"], MAX_MESSAGE_LEN)
+			var/content = reject_bad_name(params["content"], allow_numbers = TRUE, max_length = MAX_MESSAGE_LEN, strict = TRUE, cap_after_symbols = FALSE)
+			if(!content)
+				return FALSE
 
 			var/datum/medical_note/new_note = new(usr.name, content)
 			while(length(target.medical_notes) > 2)

diff --git a/code/game/machinery/computer/records/records.dm b/code/game/machinery/computer/records/records.dm
@@ -35,9 +35,9 @@
 			if(!field || !(field in target?.vars))
 				return FALSE
 
-			var/value = trim(params["value"], MAX_BROADCAST_LEN)
-			investigate_log("[key_name(user)] changed the field: \"[field]\" with value: \"[target.vars[field]]\" to new value: \"[value || "Unknown"]\"", INVESTIGATE_RECORDS)
-			target.vars[field] = value || "Unknown"
+			var/value = reject_bad_name(params["value"], allow_numbers = TRUE, max_length = MAX_BROADCAST_LEN, strict = TRUE, cap_after_symbols = FALSE) || "Unknown"
+			investigate_log("[key_name(user)] changed the field: \"[field]\" with value: \"[target.vars[field]]\" to new value: \"[value]\"", INVESTIGATE_RECORDS)
+			target.vars[field] = value
 
 			return TRUE
 

diff --git a/code/game/machinery/computer/records/security.dm b/code/game/machinery/computer/records/security.dm
@@ -177,7 +177,7 @@
 			return TRUE
 
 		if("set_note")
-			var/note = trim(params["note"], MAX_MESSAGE_LEN)
+			var/note = strip_html_full(params["note"], MAX_MESSAGE_LEN)
 			investigate_log("[user] has changed the security note of record: \"[target]\" from \"[target.security_note]\" to \"[note]\".")
 			target.security_note = note
 			return TRUE
@@ -198,7 +198,7 @@
 
 /// Handles adding a crime to a particular record.
 /obj/machinery/computer/records/security/proc/add_crime(mob/user, datum/record/crew/target, list/params)
-	var/input_name = trim(params["name"], MAX_CRIME_NAME_LEN)
+	var/input_name = strip_html_full(params["name"], MAX_CRIME_NAME_LEN)
 	if(!input_name)
 		to_chat(usr, span_warning("You must enter a name for the crime."))
 		playsound(src, 'sound/machines/terminal_error.ogg', 75, TRUE)
@@ -212,7 +212,7 @@
 
 	var/input_details
 	if(params["details"])
-		input_details = trim(params["details"], MAX_MESSAGE_LEN)
+		input_details = strip_html_full(params["details"], MAX_MESSAGE_LEN)
 
 	if(params["fine"] == 0)
 		var/datum/crime/new_crime = new(name = input_name, details = input_details, author = usr)
@@ -241,11 +241,11 @@
 		return FALSE
 
 	if(params["name"] && length(params["name"]) > 2 && params["name"] != editing_crime.name)
-		editing_crime.name = trim(params["name"], MAX_CRIME_NAME_LEN)
+		editing_crime.name = strip_html_full(params["name"], MAX_CRIME_NAME_LEN)
 		return TRUE
 
 	if(params["details"] && length(params["description"]) > 2 && params["name"] != editing_crime.name)
-		editing_crime.details = trim(params["details"], MAX_MESSAGE_LEN)
+		editing_crime.details = strip_html_full(params["details"], MAX_MESSAGE_LEN)
 		return TRUE
 
 	return FALSE
@@ -318,9 +318,9 @@
 	playsound(src, 'sound/machines/printer.ogg', 100, TRUE)
 
 	var/obj/item/printable
-	var/input_alias = trim(params["alias"], MAX_NAME_LEN) || target.name
-	var/input_description = trim(params["desc"], MAX_BROADCAST_LEN) || "No further details."
-	var/input_header = trim(params["head"], 8) || capitalize(params["type"])
+	var/input_alias = strip_html_full(params["alias"], MAX_NAME_LEN) || target.name
+	var/input_description = strip_html_full(params["desc"], MAX_BROADCAST_LEN) || "No further details."
+	var/input_header = strip_html_full(params["head"], 8) || capitalize(params["type"])
 
 	switch(params["type"])
 		if("missing")

diff --git a/code/modules/modular_computers/computers/item/computer.dm b/code/modules/modular_computers/computers/item/computer.dm
@@ -500,7 +500,7 @@
 	if(QDELETED(loc) || QDELETED(origin) || !origin.alert_able || origin.alert_silenced || !alerttext) //Yeah, we're checking alert_able. No, you don't get to make alerts that the user can't silence.
 		return FALSE
 	playsound(src, sound, 50, TRUE)
-	loc.visible_message(span_notice("<img class='icon' src='\ref[src]'> \The [src] displays a [origin.filedesc] notification: [alerttext]"), vision_distance = vision_distance, push_appearance = src)
+	loc.visible_message(span_notice("<img class='icon' src='\ref[src]'> \The [src] displays a [origin.filedesc] notification: [html_encode(alerttext)]"), vision_distance = vision_distance, push_appearance = src)
 
 /obj/item/modular_computer/proc/ring(ringtone, list/balloon_alertees) // bring bring
 	if(HAS_TRAIT(SSstation, STATION_TRAIT_PDA_GLITCHED))

diff --git a/code/modules/wiremod/core/integrated_circuit.dm b/code/modules/wiremod/core/integrated_circuit.dm
@@ -704,13 +704,13 @@ GLOBAL_LIST_EMPTY_TYPED(integrated_circuits, /obj/item/integrated_circuit)
 
 /// Sets the display name that appears on the shell.
 /obj/item/integrated_circuit/proc/set_display_name(new_name)
-	display_name = copytext(new_name, 1, label_max_length)
+	display_name = strip_html(copytext_char(new_name, 1, label_max_length)) // MONKESTATION EDIT
 	if(!shell)
 		return
 
 	if(display_name != "")
 		if(!admin_only)
-			shell.name = "[initial(shell.name)] ([strip_html(display_name)])"
+			shell.name = "[initial(shell.name)] ([display_name])" // MONKESTATION EDIT
 		else
 			shell.name = display_name
 	else

diff --git a/monkestation/code/modules/mech_comp/objects/messages/_messaging.dm b/monkestation/code/modules/mech_comp/objects/messages/_messaging.dm
@@ -13,7 +13,7 @@
 /obj/item/mcobject/messaging/examine(mob/user)
 	. = ..()
 	if(configs[MC_CFG_OUTPUT_MESSAGE])
-		. += span_notice("Output message: [stored_message]")
+		. += span_notice("Output message: [html_encode(stored_message)]")
 
 /obj/item/mcobject/messaging/proc/set_output(mob/user, obj/item/tool)
 	var/msg = input(user, "Enter new message:", "Configure Component", stored_message)

diff --git a/monkestation/code/modules/mech_comp/objects/messages/association.dm b/monkestation/code/modules/mech_comp/objects/messages/association.dm
@@ -105,7 +105,7 @@
 			else
 				mylist[ikey] = "[mylist[ikey]],[jointext(ivalue, ",")]"
 
-	to_chat(user, span_notice("You set the value of [src]'s [ikey] to [ivalue]"))
+	to_chat(user, span_notice("You set the value of [src]'s [html_encode(ikey)] to [html_encode(ivalue)]"))
 	return TRUE
 
 /obj/item/mcobject/messaging/association/proc/remove_element_config(mob/user, obj/item/tool)
@@ -117,7 +117,7 @@
 	if(isnull(removal))
 		return
 
-	to_chat(user, span_notice("You remove [src]'s [removal]:[mylist[removal]] pair."))
+	to_chat(user, span_notice("You remove [src]'s [html_encode(removal)]:[html_encode(mylist[removal])] pair."))
 	mylist -= removal
 	return TRUE
 

diff --git a/monkestation/code/modules/mech_comp/objects/messages/dispatch.dm b/monkestation/code/modules/mech_comp/objects/messages/dispatch.dm
@@ -51,7 +51,7 @@
 		return
 
 	outgoing_filters[output.interface] = splittext(filter, ",")
-	to_chat(user, span_notice("[src] will only pass messages that [exact_match ? "match" : "contain"] [filter] to [output]."))
+	to_chat(user, span_notice("[src] will only pass messages that [exact_match ? "match" : "contain"] [html_encode(filter)] to [output]."))
 
 /obj/item/mcobject/messaging/dispatch/proc/remove_message_filter(datum/mcinterface/source, datum/mcinterface/target)
 	SIGNAL_HANDLER

diff --git a/monkestation/code/modules/mech_comp/objects/messages/toggle.dm b/monkestation/code/modules/mech_comp/objects/messages/toggle.dm
@@ -10,8 +10,8 @@
 /obj/item/mcobject/messaging/toggle/examine(mob/user)
 	. = ..()
 	. += span_notice("Currently [on ? "ON":"OFF"]")
-	. += span_notice("Current ON Message: [on_signal]")
-	. += span_notice("Current OFF Message: [off_signal]")
+	. += span_notice("Current ON Message: [html_encode(on_signal)]")
+	. += span_notice("Current OFF Message: [html_encode(off_signal)]")
 
 /obj/item/mcobject/messaging/toggle/Initialize(mapload)
 	. = ..()

diff --git a/monkestation/code/modules/virology/machines/disease_records.dm b/monkestation/code/modules/virology/machines/disease_records.dm
@@ -53,7 +53,13 @@
 
 	switch(action)
 		if("edit_field")
-			target.fields[params["field"]] = params["value"]
+			var/field = params["field"]
+			if(!field || !(field in target?.fields))
+				return FALSE
+
+			var/value = reject_bad_name(params["value"], allow_numbers = TRUE, max_length = MAX_BROADCAST_LEN, strict = TRUE, cap_after_symbols = FALSE) || "Unknown"
+			target.fields[field] = value
+
 			return TRUE
 		if("expunge_record")
 			GLOB.virusDB[params["crew_ref"]] = null