From 279ee1dc12aa363582e93c154c87183707ee9e61 Mon Sep 17 00:00:00 2001
From: AffectedArc07 <25063394+AffectedArc07@users.noreply.github.com>
Date: Sat, 10 Sep 2022 14:12:16 +0100
Subject: [PATCH] Fixes a ton of security issues (#19005)
---
code/__HELPERS/_string_lists.dm | 2 +-
code/__HELPERS/files.dm | 23 ++++++++++++++++++-
code/controllers/subsystem/mapping.dm | 4 ++--
code/datums/helper_datums/map_template.dm | 2 +-
code/modules/admin/admin_investigate.dm | 2 +-
code/modules/admin/outfits.dm | 2 +-
code/modules/admin/verbs/getlogs.dm | 6 ++---
code/modules/awaymissions/maploader/reader.dm | 4 ++--
code/modules/awaymissions/maploader/writer.dm | 2 +-
code/modules/instruments/songs/play_legacy.dm | 2 +-
.../mob/living/simple_animal/friendly/dog.dm | 2 +-
code/modules/tooltip/tooltip.dm | 2 +-
goon/code/datums/browserOutput.dm | 2 +-
13 files changed, 38 insertions(+), 17 deletions(-)
diff --git a/code/__HELPERS/_string_lists.dm b/code/__HELPERS/_string_lists.dm
index e2f0c6081e7..d03c739b060 100644
--- a/code/__HELPERS/_string_lists.dm
+++ b/code/__HELPERS/_string_lists.dm
@@ -1,6 +1,6 @@
#define pick_list(FILE, KEY) (pick(strings(FILE, KEY)))
#define pick_list_replacements(FILE, KEY) (strings_replacement(FILE, KEY))
-#define json_load(FILE) (json_decode(file2text(FILE)))
+#define json_load(FILE) (json_decode(wrap_file2text(FILE)))
GLOBAL_LIST_EMPTY(string_cache)
GLOBAL_LIST_EMPTY(string_filename_current_key)
diff --git a/code/__HELPERS/files.dm b/code/__HELPERS/files.dm
index 04330b7805c..f14c9e0a7de 100644
--- a/code/__HELPERS/files.dm
+++ b/code/__HELPERS/files.dm
@@ -1,3 +1,24 @@
+// Security helpers to ensure you cant arbitrarily load stuff from disk
+/proc/wrap_file(filepath)
+ if(IsAdminAdvancedProcCall())
+ // Admins shouldnt fuck with this
+ to_chat(usr, "File load blocked: Advanced ProcCall detected.")
+ message_admins("[key_name(usr)] attempted to load files via advanced proc-call")
+ log_admin("[key_name(usr)] attempted to load files via advanced proc-call")
+ return
+
+ return file(filepath)
+
+/proc/wrap_file2text(filepath)
+ if(IsAdminAdvancedProcCall())
+ // Admins shouldnt fuck with this
+ to_chat(usr, "File load blocked: Advanced ProcCall detected.")
+ message_admins("[key_name(usr)] attempted to load files via advanced proc-call")
+ log_admin("[key_name(usr)] attempted to load files via advanced proc-call")
+ return
+
+ return file2text(filepath)
+
//checks if a file exists and contains text
//returns text as a string if these conditions are met
/proc/return_file_text(filename)
@@ -5,7 +26,7 @@
error("File not found ([filename])")
return
- var/text = file2text(filename)
+ var/text = wrap_file2text(filename)
if(!text)
error("File empty ([filename])")
return
diff --git a/code/controllers/subsystem/mapping.dm b/code/controllers/subsystem/mapping.dm
index 06a96c50103..dabe5aca469 100644
--- a/code/controllers/subsystem/mapping.dm
+++ b/code/controllers/subsystem/mapping.dm
@@ -151,7 +151,7 @@ SUBSYSTEM_DEF(mapping)
log_startup_progress("Loading [map_datum.fluff_name]...")
// This should always be Z2, but you never know
var/map_z_level = GLOB.space_manager.add_new_zlevel(MAIN_STATION, linkage = CROSSLINKED, traits = list(STATION_LEVEL, STATION_CONTACT, REACHABLE, AI_OK))
- GLOB.maploader.load_map(file(map_datum.map_path), z_offset = map_z_level)
+ GLOB.maploader.load_map(wrap_file(map_datum.map_path), z_offset = map_z_level)
log_startup_progress("Loaded [map_datum.fluff_name] in [stop_watch(watch)]s")
// Save station name in the DB
@@ -265,7 +265,7 @@ SUBSYSTEM_DEF(mapping)
log_startup_progress("Loading away mission...")
var/map = pick(GLOB.configuration.gateway.enabled_away_missions)
- var/file = file(map)
+ var/file = wrap_file(map)
if(isfile(file))
var/zlev = GLOB.space_manager.add_new_zlevel(AWAY_MISSION, linkage = UNAFFECTED, traits = list(AWAY_LEVEL,BLOCK_TELEPORT))
GLOB.space_manager.add_dirt(zlev)
diff --git a/code/datums/helper_datums/map_template.dm b/code/datums/helper_datums/map_template.dm
index 01d01dd1423..0d44c3aa3c8 100644
--- a/code/datums/helper_datums/map_template.dm
+++ b/code/datums/helper_datums/map_template.dm
@@ -72,7 +72,7 @@
if(mapfile)
. = mapfile
else if(mappath)
- . = file(mappath)
+ . = wrap_file(mappath)
if(!.)
stack_trace(" The file of [src] appears to be empty/non-existent.")
diff --git a/code/modules/admin/admin_investigate.dm b/code/modules/admin/admin_investigate.dm
index db38844c40c..eb9f75ddf9f 100644
--- a/code/modules/admin/admin_investigate.dm
+++ b/code/modules/admin/admin_investigate.dm
@@ -9,7 +9,7 @@
//SYSTEM
/proc/investigate_subject2file(subject)
- return file("[INVESTIGATE_DIR][subject].html")
+ return wrap_file("[INVESTIGATE_DIR][subject].html")
/proc/investigate_reset()
if(fdel(INVESTIGATE_DIR)) return 1
diff --git a/code/modules/admin/outfits.dm b/code/modules/admin/outfits.dm
index 336edc89c88..da4b042e755 100644
--- a/code/modules/admin/outfits.dm
+++ b/code/modules/admin/outfits.dm
@@ -35,7 +35,7 @@ GLOBAL_LIST_EMPTY(custom_outfits) //Admin created outfits
var/outfit_file = input("Pick outfit json file:", "File") as null|file
if(!outfit_file)
return
- var/filedata = file2text(outfit_file)
+ var/filedata = wrap_file2text(outfit_file)
var/json = json_decode(filedata)
if(!json)
to_chat(admin,"JSON decode error.")
diff --git a/code/modules/admin/verbs/getlogs.dm b/code/modules/admin/verbs/getlogs.dm
index 73f993c61de..88bcc889a22 100644
--- a/code/modules/admin/verbs/getlogs.dm
+++ b/code/modules/admin/verbs/getlogs.dm
@@ -30,11 +30,11 @@
message_admins("[key_name_admin(src)] accessed file: [path]")
switch(alert("View (in game), Open (in your system's text editor), or Download?", path, "View", "Open", "Download"))
if ("View")
- src << browse("[html_encode(file2text(file(path)))]
", list2params(list("window" = "viewfile.[path]")))
+ src << browse("[html_encode(wrap_file2text(wrap_file(path)))]
", list2params(list("window" = "viewfile.[path]")))
if ("Open")
- src << run(file(path))
+ src << run(wrap_file(path))
if ("Download")
- src << ftp(file(path))
+ src << ftp(wrap_file(path))
else
return
to_chat(src, "Attempting to send [path], this may take a fair few minutes if the file is very large.")
diff --git a/code/modules/awaymissions/maploader/reader.dm b/code/modules/awaymissions/maploader/reader.dm
index 336c84b9655..de15e7bbc4d 100644
--- a/code/modules/awaymissions/maploader/reader.dm
+++ b/code/modules/awaymissions/maploader/reader.dm
@@ -35,7 +35,7 @@ GLOBAL_DATUM_INIT(_preloader, /datum/dmm_suite/preloader, new())
if(lastchar == "/" || lastchar == "\\")
log_debug("Attempted to load map template without filename (Attempted [tfile])")
return
- tfile = file2text(tfile)
+ tfile = wrap_file2text(tfile)
if(!length(tfile))
throw EXCEPTION("Map path '[fname]' does not exist!")
@@ -407,7 +407,7 @@ GLOBAL_DATUM_INIT(_preloader, /datum/dmm_suite/preloader, new())
// Check for file
else if(copytext(value_text, 1, 2) == "'")
- . = file(copytext(value_text, 2, length(value_text)))
+ . = wrap_file(copytext(value_text, 2, length(value_text)))
// Check for path
else if(ispath(text2path(value_text)))
diff --git a/code/modules/awaymissions/maploader/writer.dm b/code/modules/awaymissions/maploader/writer.dm
index 91dcb8b83d1..23b0a735714 100644
--- a/code/modules/awaymissions/maploader/writer.dm
+++ b/code/modules/awaymissions/maploader/writer.dm
@@ -18,7 +18,7 @@
var/map_path = "[map_prefix][map_name].dmm"
if(fexists(map_path))
fdel(map_path)
- var/saved_map = file(map_path)
+ var/saved_map = wrap_file(map_path)
var/map_text = write_map(t1, t2, flags, saved_map)
saved_map << map_text
return saved_map
diff --git a/code/modules/instruments/songs/play_legacy.dm b/code/modules/instruments/songs/play_legacy.dm
index 34ca00640a3..fc3d0151564 100644
--- a/code/modules/instruments/songs/play_legacy.dm
+++ b/code/modules/instruments/songs/play_legacy.dm
@@ -71,7 +71,7 @@
// now generate name
var/filename = "sound/instruments/[cached_legacy_dir]/[ascii2text(note + 64)][acc][oct].[cached_legacy_ext]"
- var/soundfile = file(filename)
+ var/soundfile = wrap_file(filename)
// make sure the note exists
var/cached_fexists = valid_files[filename]
if(!isnull(cached_fexists))
diff --git a/code/modules/mob/living/simple_animal/friendly/dog.dm b/code/modules/mob/living/simple_animal/friendly/dog.dm
index aaa19d2cedd..2734d4ad86f 100644
--- a/code/modules/mob/living/simple_animal/friendly/dog.dm
+++ b/code/modules/mob/living/simple_animal/friendly/dog.dm
@@ -443,7 +443,7 @@
var/json_file = file("data/npc_saves/Ian.json")
if(!fexists(json_file))
return
- var/list/json = json_decode(file2text(json_file))
+ var/list/json = json_decode(wrap_file2text(json_file))
age = json["age"]
record_age = json["record_age"]
saved_head = json["saved_head"]
diff --git a/code/modules/tooltip/tooltip.dm b/code/modules/tooltip/tooltip.dm
index 8aebf1a2f1f..842d83c3883 100644
--- a/code/modules/tooltip/tooltip.dm
+++ b/code/modules/tooltip/tooltip.dm
@@ -43,7 +43,7 @@ Notes:
/datum/tooltip/New(client/C)
if(C)
owner = C
- owner << browse(file2text(file), "window=[control]")
+ owner << browse(wrap_file2text(file), "window=[control]")
..()
diff --git a/goon/code/datums/browserOutput.dm b/goon/code/datums/browserOutput.dm
index 61af3a6e378..a4522042597 100644
--- a/goon/code/datums/browserOutput.dm
+++ b/goon/code/datums/browserOutput.dm
@@ -66,7 +66,7 @@ var/list/chatResources = list(
for(var/attempts in 1 to 5)
for(var/asset in global.chatResources)
- owner << browse_rsc(file(asset))
+ owner << browse_rsc(wrap_file(asset))
for(var/subattempts in 1 to 3)
owner << browse(file2text("goon/browserassets/html/browserOutput.html"), "window=browseroutput")