From 08ddf2a09e4be404c4780ceaf5575e940449ef1f Mon Sep 17 00:00:00 2001
From: SkyratBot <59378654+SkyratBot@users.noreply.github.com>
Date: Wed, 12 Jul 2023 05:58:51 +0200
Subject: [PATCH] [MIRROR] Small security / code passover for radar mod pc
 program  [MDB IGNORE] (#22104)

* Small security / code passover for radar mod pc program  (#76299)

## About The Pull Request

I saw this line "randomly" and thought "well that can't be safe"

https://github.com/tgstation/tgstation/blob/971fbf93629ff48d39302544889060ba69fa0ccc/code/modules/modular_computers/file_system/programs/radar.dm#L67-L68

So I went through and added a sanity check for it to ensure it's only
getting passed `ref`s being tracked in the objects list

This accompanied some general code improvements

## Why It's Good For The Game

Just some extra safety
Some of these apps do a follow up check that the selected ref is a valid
ref to track, but not all of them (notably the Fission 360). It's
unlikely this can be abused but it's good to have

## Changelog

:cl: Melbert
qol: Scanning apps (Lifeline app) start with a scan
fix: Fixes a potential exploit in radar apps
/:cl:

* Small security / code passover for radar mod pc program

---------

Co-authored-by: MrMelbert <51863163+MrMelbert@users.noreply.github.com>
Co-authored-by: Bloop <vinylspiders@gmail.com>
---
 .../file_system/programs/radar.dm             | 54 ++++++++++---------
 1 file changed, 29 insertions(+), 25 deletions(-)

diff --git a/code/modules/modular_computers/file_system/programs/radar.dm b/code/modules/modular_computers/file_system/programs/radar.dm
index 85dfebcd7e7..2298daa8d37 100644
--- a/code/modules/modular_computers/file_system/programs/radar.dm
+++ b/code/modules/modular_computers/file_system/programs/radar.dm
@@ -10,11 +10,11 @@
 	size = 5
 	tgui_id = "NtosRadar"
 	///List of trackable entities. Updated by the scan() proc.
-	var/list/objects
+	var/list/list/objects
 	///Ref of the last trackable object selected by the user in the tgui window. Updated in the ui_act() proc.
-	var/atom/selected
-	///Used to store when the next scan is available. Updated by the scan() proc.
-	var/next_scan = 0
+	var/selected
+	///Used to store when the next scan is available.
+	COOLDOWN_DECLARE(next_scan)
 	///Used to keep track of the last value program_icon_state was set to, to prevent constant unnecessary update_appearance() calls
 	var/last_icon_state = ""
 	///Used by the tgui interface, themed NT or Syndicate.
@@ -24,10 +24,12 @@
 
 /datum/computer_file/program/radar/on_start(mob/living/user)
 	. = ..()
-	if(.)
-		START_PROCESSING(SSfastprocess, src)
+	if(!.)
 		return
-	return FALSE
+	if(COOLDOWN_FINISHED(src, next_scan))
+		// start with a scan without a cooldown, but don't scan if we *are* on cooldown already.
+		scan()
+	START_PROCESSING(SSfastprocess, src)
 
 /datum/computer_file/program/radar/kill_program()
 	objects = list()
@@ -47,14 +49,8 @@
 /datum/computer_file/program/radar/ui_data(mob/user)
 	var/list/data = list()
 	data["selected"] = selected
-	data["objects"] = list()
-	data["scanning"] = (world.time < next_scan)
-	for(var/list/i in objects)
-		var/list/objectdata = list(
-			ref = i["ref"],
-			name = i["name"],
-		)
-		data["object"] += list(objectdata)
+	data["scanning"] = !COOLDOWN_FINISHED(src, next_scan)
+	data["object"] = objects
 
 	data["target"] = list()
 	var/list/trackinfo = track()
@@ -63,11 +59,28 @@
 	return data
 
 /datum/computer_file/program/radar/ui_act(action, params, datum/tgui/ui, datum/ui_state/state)
+
 	switch(action)
 		if("selecttarget")
-			selected = params["ref"]
+			var/selected_new_ref = params["ref"]
+			if(selected_new_ref in trackable_object_refs())
+				selected = selected_new_ref
+			return TRUE
+
 		if("scan")
+			if(!COOLDOWN_FINISHED(src, next_scan))
+				return TRUE // update anyways
+
+			COOLDOWN_START(src, next_scan, 2 SECONDS)
 			scan()
+			return TRUE
+
+/// Returns all ref()s that are being tracked currently
+/datum/computer_file/program/radar/proc/trackable_object_refs()
+	var/list/all_refs = list()
+	for(var/list/object_list as anything in objects)
+		all_refs += object_list["ref"]
+	return all_refs
 
 /**
  *Updates tracking information of the selected target.
@@ -215,9 +228,6 @@
 	return locate(selected) in GLOB.human_list
 
 /datum/computer_file/program/radar/lifeline/scan()
-	if(world.time < next_scan)
-		return
-	next_scan = world.time + (2 SECONDS)
 	objects = list()
 	for(var/i in GLOB.human_list)
 		var/mob/living/carbon/human/humanoid = i
@@ -260,9 +270,6 @@
 	return locate(selected) in GLOB.janitor_devices
 
 /datum/computer_file/program/radar/custodial_locator/scan()
-	if(world.time < next_scan)
-		return
-	next_scan = world.time + (2 SECONDS)
 	objects = list()
 	for(var/obj/custodial_tools as anything in GLOB.janitor_devices)
 		if(!trackable(custodial_tools))
@@ -325,9 +332,6 @@
 	return SSpoints_of_interest.get_poi_atom_by_ref(selected)
 
 /datum/computer_file/program/radar/fission360/scan()
-	if(world.time < next_scan)
-		return
-	next_scan = world.time + (2 SECONDS)
 	objects = list()
 
 	// All the nukes