From cc0179522bb7ceec7ddb84437cee69fa579ab3d1 Mon Sep 17 00:00:00 2001
From: Mothblocks <35135081+Mothblocks@users.noreply.github.com>
Date: Sun, 21 Aug 2022 21:01:47 -0700
Subject: [PATCH] Check for compliant JSON before receiving payload (#69023)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

About The Pull Request

This was denied in the past for perf reasons, but I want to test how much this actually reduces perf.

rustg_json_is_valid checks for two things.

    It checks that the data isn't too nested. This could easily be removed if it causes perf problems, since it's an extra O(n) check of its own. It was for a BYOND exploit that is now fixed.
    It checks for JSON compliancy. It is backed by serde_json, which does not allow NaN, whereas BYOND is incompliant.

This does not lady_beetle ribbon 𝒻𝒾𝓍 ribbon lady_beetle #69017, because that is doing an extra text2num for no reason. If it didn't (like it shouldn't), then this would fix it.

Will be test merged on Manuel but not Sybil--CC @LemonInTheDark help me on figuring out the perf cost of this. If it's noticable then we can test with removing the depth check.
---
 code/modules/tgui/external.dm | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/code/modules/tgui/external.dm b/code/modules/tgui/external.dm
index 6af2f94d2cb..b49baf14996 100644
--- a/code/modules/tgui/external.dm
+++ b/code/modules/tgui/external.dm
@@ -218,10 +218,18 @@
 				context = window_id)
 			SStgui.force_close_window(usr, window_id)
 			return TRUE
+
 	// Decode payload
 	var/payload
 	if(href_list["payload"])
-		payload = json_decode(href_list["payload"])
+		var/payload_text = href_list["payload"]
+
+		if (!rustg_json_is_valid(payload_text))
+			log_tgui(usr, "Error: Invalid JSON")
+			return TRUE
+
+		payload = json_decode(payload_text)
+
 	// Pass message to window
 	if(window)
 		window.on_message(type, payload, href_list)