From 76042de2b693d6a10b61bcd88f7ccd955149deae Mon Sep 17 00:00:00 2001 From: Timberpoes Date: Sun, 7 Feb 2021 11:35:36 +0000 Subject: [PATCH] Improve ID console name and assignment input (#56662) reject_bad_name and reject_bad_text doesn't directly sanitize. It's best not to rely on this proc to sanitize as that's not the point of it, even though it can sanitize as part of its functioning. Instead, we explicitly sanitize the input first, then we reject_bad_whatever on the sanitized input. It also offers improved feedback to the user instead of failing silently, and allows numbers as some of our ID cards (For example, prisoner IDs) have numbers in their registered_names and assignments. I don't like creating the game world in a state that players cannot also replicate themselves. Also swings by the ancient HTML interfaces to bring them up to the same standard through either sanitizing input or stripping input as appropriate. --- code/game/machinery/computer/card.dm | 14 +++++++++---- .../file_system/programs/card.dm | 20 ++++++++++++++++--- 2 files changed, 27 insertions(+), 7 deletions(-) diff --git a/code/game/machinery/computer/card.dm b/code/game/machinery/computer/card.dm index d9e0202e198..5e3451fda32 100644 --- a/code/game/machinery/computer/card.dm +++ b/code/game/machinery/computer/card.dm @@ -455,7 +455,7 @@ GLOBAL_VAR_INIT(time_last_changed_position, 0) if (authenticated == 2) var/t1 = href_list["assign_target"] if(t1 == "Custom") - var/newJob = reject_bad_text(input("Enter a custom job assignment.", "Assignment", inserted_modify_id ? inserted_modify_id.assignment : "Unassigned"), MAX_NAME_LEN) + var/newJob = reject_bad_text(stripped_input("Enter a custom job assignment.", "Assignment", inserted_modify_id ? inserted_modify_id.assignment : "Unassigned"), MAX_NAME_LEN) if(newJob) t1 = newJob @@ -509,9 +509,15 @@ GLOBAL_VAR_INIT(time_last_changed_position, 0) to_chat(usr, "Invalid age entered- age not updated.") updateUsrDialog() - var/newName = reject_bad_name(href_list["reg"]) - if(newName) - inserted_modify_id.registered_name = newName + + // Sanitize the name first. We're not using the full sanitize_name proc as ID cards can have a wider variety of things on them that + // would not pass as a formal character name, but would still be valid on an ID card created by a player. + var/new_name = sanitize(href_list["reg"]) + // However, we are going to reject bad names overall including names with invalid characters in them, while allowing numbers. + new_name = reject_bad_name(new_name, allow_numbers = TRUE) + + if(new_name) + inserted_modify_id.registered_name = new_name playsound(src, 'sound/machines/terminal_prompt_confirm.ogg', 50, FALSE) else to_chat(usr, "Invalid name entered.") diff --git a/code/modules/modular_computers/file_system/programs/card.dm b/code/modules/modular_computers/file_system/programs/card.dm index a0575869b40..c97bac6473a 100644 --- a/code/modules/modular_computers/file_system/programs/card.dm +++ b/code/modules/modular_computers/file_system/programs/card.dm @@ -177,9 +177,17 @@ if("PRG_edit") if(!computer || !authenticated || !target_id_card) return - var/new_name = reject_bad_name(params["name"]) // if reject bad name fails, the edit will just not go through instead of discarding all input, as new_name would be blank. + + // Sanitize the name first. We're not using the full sanitize_name proc as ID cards can have a wider variety of things on them that + // would not pass as a formal character name, but would still be valid on an ID card created by a player. + var/new_name = sanitize(params["name"]) + // However, we are going to reject bad names overall including names with invalid characters in them, while allowing numbers. + new_name = reject_bad_name(new_name, allow_numbers = TRUE) + if(!new_name) + to_chat(usr, "Software error: The ID card rejected the new name as it contains prohibited characters.") return + target_id_card.registered_name = new_name target_id_card.update_label() playsound(computer, "terminal_type", 50, FALSE) @@ -192,8 +200,14 @@ return if(target == "Custom") - var/custom_name = reject_bad_name(params["custom_name"]) // if reject bad name fails, the edit will just not go through, as custom_name would be empty - if(custom_name) + // Sanitize the custom assignment name first. + var/custom_name = sanitize(params["custom_name"]) + // However, we are going to assignments containing bad text overall. + custom_name = reject_bad_text(custom_name) + + if(!custom_name) + to_chat(usr, "Software error: The ID card rejected the new custom assignment as it contains prohibited characters.") + else target_id_card.assignment = custom_name target_id_card.update_label() else