From 608c71c6e59639d251c0d3344324ec9f3bea478b Mon Sep 17 00:00:00 2001 From: AffectedArc07 Date: Thu, 9 Jul 2020 10:29:51 +0100 Subject: [PATCH] Adds proc call security --- code/__HELPERS/unsorted.dm | 3 + code/controllers/configuration.dm | 10 +++ code/datums/datumvars.dm | 9 +++ code/defines/procs/dbcore.dm | 9 +++ code/modules/admin/admin_ranks.dm | 5 ++ code/modules/admin/holder2.dm | 25 +++++++ .../admin/permissionverbs/permissionedit.dm | 5 ++ code/modules/admin/verbs/SDQL2/SDQL_2.dm | 4 +- code/modules/admin/verbs/debug.dm | 66 ++++++++++++++++++- code/modules/research/message_server.dm | 30 +++++++++ 10 files changed, 161 insertions(+), 5 deletions(-) diff --git a/code/__HELPERS/unsorted.dm b/code/__HELPERS/unsorted.dm index 63d9f7c85ef..7e57a394988 100644 --- a/code/__HELPERS/unsorted.dm +++ b/code/__HELPERS/unsorted.dm @@ -2016,3 +2016,6 @@ GLOBAL_DATUM_INIT(dview_mob, /mob/dview, new) /proc/CallAsync(datum/source, proctype, list/arguments) set waitfor = FALSE return call(source, proctype)(arglist(arguments)) + +/// Waits at a line of code until X is true +#define UNTIL(X) while(!(X)) stoplag() diff --git a/code/controllers/configuration.dm b/code/controllers/configuration.dm index 083b347e52c..9d4dacf33f2 100644 --- a/code/controllers/configuration.dm +++ b/code/controllers/configuration.dm @@ -265,6 +265,11 @@ src.votable_modes += "secret" /datum/configuration/proc/load(filename, type = "config") //the type can also be game_options, in which case it uses a different switch. not making it separate to not copypaste code - Urist + if(IsAdminAdvancedProcCall()) + to_chat(usr, "Config reload blocked: Advanced ProcCall detected.") + message_admins("[key_name(usr)] attempted to reload configuration via advanced proc-call") + log_admin("[key_name(usr)] attempted to reload configuration via advanced proc-call") + return var/list/Lines = file2list(filename) for(var/t in Lines) @@ -808,6 +813,11 @@ log_config("Unknown setting in configuration: '[name]'") /datum/configuration/proc/loadsql(filename) // -- TLE + if(IsAdminAdvancedProcCall()) + to_chat(usr, "SQL configuration reload blocked: Advanced ProcCall detected.") + message_admins("[key_name(usr)] attempted to reload SQL configuration via advanced proc-call") + log_admin("[key_name(usr)] attempted to reload SQL configuration via advanced proc-call") + return var/list/Lines = file2list(filename) for(var/t in Lines) if(!t) continue diff --git a/code/datums/datumvars.dm b/code/datums/datumvars.dm index e8d0b881ccb..ad6a9eabbba 100644 --- a/code/datums/datumvars.dm +++ b/code/datums/datumvars.dm @@ -1,5 +1,14 @@ // reference: /client/proc/modify_variables(var/atom/O, var/param_var_name = null, var/autodetect_class = 0) +/** + * Proc to check if a datum allows proc calls on it + * + * Returns TRUE if you can call a proc on the datum, FALSE if you cant + * + */ +/datum/proc/CanProcCall(procname) + return TRUE + /datum/proc/can_vv_get(var_name) return TRUE diff --git a/code/defines/procs/dbcore.dm b/code/defines/procs/dbcore.dm index 76e5a1fd0a6..90a5b0db40c 100644 --- a/code/defines/procs/dbcore.dm +++ b/code/defines/procs/dbcore.dm @@ -72,12 +72,21 @@ DBConnection/proc/NewQuery(sql_query,cursor_handler=src.default_cursor) return n DBQuery/New(sql_query,DBConnection/connection_handler,cursor_handler) + if(IsAdminAdvancedProcCall()) + to_chat(usr, "DB query blocked: Advanced ProcCall detected.") + message_admins("[key_name(usr)] attempted to create a DB query via advanced proc-call") + log_admin("[key_name(usr)] attempted to create a DB query via advanced proc-call") + return if(sql_query) src.sql = sql_query if(connection_handler) src.db_connection = connection_handler if(cursor_handler) src.default_cursor = cursor_handler _db_query = _dm_db_new_query() return ..() +DBQuery/CanProcCall() + // dont even try it + return FALSE + DBQuery var/sql // The sql query being executed. diff --git a/code/modules/admin/admin_ranks.dm b/code/modules/admin/admin_ranks.dm index 5956819d3d4..afb4b19eee2 100644 --- a/code/modules/admin/admin_ranks.dm +++ b/code/modules/admin/admin_ranks.dm @@ -61,6 +61,11 @@ GLOBAL_PROTECT(admin_ranks) // this shit is being protected for obvious reasons return 1 /proc/load_admins() + if(IsAdminAdvancedProcCall()) + to_chat(usr, "Admin reload blocked: Advanced ProcCall detected.") + message_admins("[key_name(usr)] attempted to reload admins via advanced proc-call") + log_admin("[key_name(usr)] attempted to reload admins via advanced proc-call") + return //clear the datums references GLOB.admin_datums.Cut() for(var/client/C in GLOB.admins) diff --git a/code/modules/admin/holder2.dm b/code/modules/admin/holder2.dm index 740d182f3f2..470293bfe76 100644 --- a/code/modules/admin/holder2.dm +++ b/code/modules/admin/holder2.dm @@ -16,6 +16,11 @@ GLOBAL_PROTECT(admin_datums) // This is protected because we dont want people ma var/admincaster_signature //What you'll sign the newsfeeds as /datum/admins/New(initial_rank = "Temporary Admin", initial_rights = 0, ckey) + if(IsAdminAdvancedProcCall()) + to_chat(usr, "Admin rank creation blocked: Advanced ProcCall detected.") + message_admins("[key_name(usr)] attempted to create a new admin rank via advanced proc-call") + log_admin("[key_name(usr)] attempted to edit feedback a new admin rank via advanced proc-call") + return if(!ckey) error("Admin datum created without a ckey argument. Datum has been deleted") qdel(src) @@ -26,10 +31,20 @@ GLOBAL_PROTECT(admin_datums) // This is protected because we dont want people ma GLOB.admin_datums[ckey] = src /datum/admins/Destroy() + if(IsAdminAdvancedProcCall()) + to_chat(usr, "Admin rank deletion blocked: Advanced ProcCall detected.") + message_admins("[key_name(usr)] attempted to delete an admin rank via advanced proc-call") + log_admin("[key_name(usr)] attempted to delete an admin rank via advanced proc-call") + return ..() return QDEL_HINT_HARDDEL_NOW /datum/admins/proc/associate(client/C) + if(IsAdminAdvancedProcCall()) + to_chat(usr, "Rank association blocked: Advanced ProcCall detected.") + message_admins("[key_name(usr)] attempted to associate an admin rank to a new client via advanced proc-call") + log_admin("[key_name(usr)] attempted to associate an admin rank to a new client via advanced proc-call") + return if(istype(C)) owner = C owner.holder = src @@ -39,6 +54,11 @@ GLOBAL_PROTECT(admin_datums) // This is protected because we dont want people ma GLOB.admins |= C /datum/admins/proc/disassociate() + if(IsAdminAdvancedProcCall()) + to_chat(usr, "Rank disassociation blocked: Advanced ProcCall detected.") + message_admins("[key_name(usr)] attempted to disassociate an admin rank from a client via advanced proc-call") + log_admin("[key_name(usr)] attempted to disassociate an admin rank from a client via advanced proc-call") + return if(owner) GLOB.admins -= owner owner.remove_admin_verbs() @@ -88,6 +108,11 @@ you will have to do something like if(client.holder.rights & R_ADMIN) yourself. return 0 /client/proc/deadmin() + if(IsAdminAdvancedProcCall()) + to_chat(usr, "Deadmin blocked: Advanced ProcCall detected.") + message_admins("[key_name(usr)] attempted to de-admin a client via advanced proc-call") + log_admin("[key_name(usr)] attempted to de-admin a client via advanced proc-call") + return GLOB.admin_datums -= ckey if(holder) holder.disassociate() diff --git a/code/modules/admin/permissionverbs/permissionedit.dm b/code/modules/admin/permissionverbs/permissionedit.dm index 795538c7c33..952630c798a 100644 --- a/code/modules/admin/permissionverbs/permissionedit.dm +++ b/code/modules/admin/permissionverbs/permissionedit.dm @@ -102,6 +102,11 @@ to_chat(usr, "Admin rank changed.") /datum/admins/proc/log_admin_permission_modification(var/adm_ckey, var/new_permission) + if(IsAdminAdvancedProcCall()) + to_chat(usr, "Admin edit blocked: Advanced ProcCall detected.") + message_admins("[key_name(usr)] attempted to edit admin ranks via advanced proc-call") + log_admin("[key_name(usr)] attempted to edit admin ranks via advanced proc-call") + return if(config.admin_legacy_system) return diff --git a/code/modules/admin/verbs/SDQL2/SDQL_2.dm b/code/modules/admin/verbs/SDQL2/SDQL_2.dm index 018b45422c5..c4eabef3eae 100644 --- a/code/modules/admin/verbs/SDQL2/SDQL_2.dm +++ b/code/modules/admin/verbs/SDQL2/SDQL_2.dm @@ -444,9 +444,9 @@ if(object == world) // Global proc. procname = "/proc/[procname]" - return call(procname)(arglist(new_args)) + return (WrapAdminProcCall(GLOBAL_PROC, procname, new_args)) - return call(object, procname)(arglist(new_args)) + return (WrapAdminProcCall(object, procname, new_args)) /proc/SDQL2_tokenize(query_text) diff --git a/code/modules/admin/verbs/debug.dm b/code/modules/admin/verbs/debug.dm index 496292c919b..3464abcb981 100644 --- a/code/modules/admin/verbs/debug.dm +++ b/code/modules/admin/verbs/debug.dm @@ -85,18 +85,78 @@ But you can call procs that are of type /mob/living/carbon/human/proc/ for that return message_admins("[key_name_admin(src)] called [target]'s [procname]() with [lst.len ? "the arguments [list2params(lst)]":"no arguments"].") log_admin("[key_name(src)] called [target]'s [procname]() with [lst.len ? "the arguments [list2params(lst)]":"no arguments"].") - returnval = call(target,procname)(arglist(lst)) // Pass the lst as an argument list to the proc + returnval = WrapAdminProcCall(target, procname, lst) // Pass the lst as an argument list to the proc else //this currently has no hascall protection. wasn't able to get it working. message_admins("[key_name_admin(src)] called [procname]() with [lst.len ? "the arguments [list2params(lst)]":"no arguments"]") log_admin("[key_name(src)] called [procname]() with [lst.len ? "the arguments [list2params(lst)]":"no arguments"]") - returnval = call(procname)(arglist(lst)) // Pass the lst as an argument list to the proc + returnval = WrapAdminProcCall(GLOBAL_PROC, procname, lst) // Pass the lst as an argument list to the proc to_chat(usr, "[procname] returned: [!isnull(returnval) ? returnval : "null"]") feedback_add_details("admin_verb","APC") //If you are copy-pasting this, ensure the 2nd parameter is unique to the new proc! +// All these vars are related to proc call protection +// If you add more of these, for the love of fuck, protect them + +/// Who is currently calling procs GLOBAL_VAR(AdminProcCaller) GLOBAL_PROTECT(AdminProcCaller) +/// How many procs have been called +GLOBAL_VAR_INIT(AdminProcCallCount, 0) +GLOBAL_PROTECT(AdminProcCallCount) +/// UID of the admin who last called +GLOBAL_VAR(LastAdminCalledTargetUID) +GLOBAL_PROTECT(LastAdminCalledTargetUID) +/// Last target to have a proc called on it +GLOBAL_VAR(LastAdminCalledTarget) +GLOBAL_PROTECT(LastAdminCalledTarget) +/// Last proc called +GLOBAL_VAR(LastAdminCalledProc) +GLOBAL_PROTECT(LastAdminCalledProc) +/// List to handle proc call spam prevention +GLOBAL_LIST_EMPTY(AdminProcCallSpamPrevention) +GLOBAL_PROTECT(AdminProcCallSpamPrevention) + + +// Wrapper for proccalls where the datum is flagged as vareditted +/proc/WrapAdminProcCall(datum/target, procname, list/arguments) + if(target && procname == "Del") + to_chat(usr, "Calling Del() is not allowed") + return + + if(target != GLOBAL_PROC && !target.CanProcCall(procname)) + to_chat(usr, "Proccall on [target.type]/proc/[procname] is disallowed!") + return + var/current_caller = GLOB.AdminProcCaller + var/ckey = usr ? usr.client.ckey : GLOB.AdminProcCaller + if(!ckey) + CRASH("WrapAdminProcCall with no ckey: [target] [procname] [english_list(arguments)]") + if(current_caller && current_caller != ckey) + if(!GLOB.AdminProcCallSpamPrevention[ckey]) + to_chat(usr, "Another set of admin called procs are still running, your proc will be run after theirs finish.") + GLOB.AdminProcCallSpamPrevention[ckey] = TRUE + UNTIL(!GLOB.AdminProcCaller) + to_chat(usr, "Running your proc") + GLOB.AdminProcCallSpamPrevention -= ckey + else + UNTIL(!GLOB.AdminProcCaller) + GLOB.LastAdminCalledProc = procname + if(target != GLOBAL_PROC) + GLOB.LastAdminCalledTargetUID = target.UID() + GLOB.AdminProcCaller = ckey //if this runtimes, too bad for you + ++GLOB.AdminProcCallCount + . = world.WrapAdminProcCall(target, procname, arguments) + if(--GLOB.AdminProcCallCount == 0) + GLOB.AdminProcCaller = null + +//adv proc call this, ya nerds +/world/proc/WrapAdminProcCall(datum/target, procname, list/arguments) + if(target == GLOBAL_PROC) + return call(procname)(arglist(arguments)) + else if(target != world) + return call(target, procname)(arglist(arguments)) + else + log_admin("[key_name(usr)] attempted to call world/proc/[procname] with arguments: [english_list(arguments)]") /proc/IsAdminAdvancedProcCall() #ifdef TESTING @@ -131,7 +191,7 @@ GLOBAL_PROTECT(AdminProcCaller) log_admin("[key_name(src)] called [A]'s [procname]() with [lst.len ? "the arguments [list2params(lst)]":"no arguments"]") spawn() - var/returnval = call(A,procname)(arglist(lst)) // Pass the lst as an argument list to the proc + var/returnval = WrapAdminProcCall(A, procname, lst) // Pass the lst as an argument list to the proc to_chat(src, "[procname] returned: [!isnull(returnval) ? returnval : "null"]") feedback_add_details("admin_verb","DPC") //If you are copy-pasting this, ensure the 2nd parameter is unique to the new proc! diff --git a/code/modules/research/message_server.dm b/code/modules/research/message_server.dm index 91306d910f1..3fe0ef54c0c 100644 --- a/code/modules/research/message_server.dm +++ b/code/modules/research/message_server.dm @@ -305,6 +305,11 @@ GLOBAL_DATUM(blackbox, /obj/machinery/blackbox_recorder) //This proc is only to be called at round end. /obj/machinery/blackbox_recorder/proc/save_all_data_to_sql() + if(IsAdminAdvancedProcCall()) + to_chat(usr, "Blackbox seal blocked: Advanced ProcCall detected.") + message_admins("[key_name(usr)] attempted to seal the blackbox via advanced proc-call") + log_admin("[key_name(usr)] attempted to seal the blackbox via advanced proc-call") + return if(!feedback) return round_end_data_gathering() //round_end time logging and some other data processing @@ -331,6 +336,11 @@ GLOBAL_DATUM(blackbox, /obj/machinery/blackbox_recorder) proc/feedback_set(var/variable,var/value) + if(IsAdminAdvancedProcCall()) + to_chat(usr, "Feedback edit blocked: Advanced ProcCall detected.") + message_admins("[key_name(usr)] attempted to edit feedback data via advanced proc-call") + log_admin("[key_name(usr)] attempted to edit feedback data via advanced proc-call") + return if(!GLOB.blackbox) return variable = sanitizeSQL(variable) @@ -342,6 +352,11 @@ proc/feedback_set(var/variable,var/value) FV.set_value(value) proc/feedback_inc(var/variable,var/value) + if(IsAdminAdvancedProcCall()) + to_chat(usr, "Feedback edit blocked: Advanced ProcCall detected.") + message_admins("[key_name(usr)] attempted to edit feedback data via advanced proc-call") + log_admin("[key_name(usr)] attempted to edit feedback data via advanced proc-call") + return if(!GLOB.blackbox) return variable = sanitizeSQL(variable) @@ -353,6 +368,11 @@ proc/feedback_inc(var/variable,var/value) FV.inc(value) proc/feedback_dec(var/variable,var/value) + if(IsAdminAdvancedProcCall()) + to_chat(usr, "Feedback edit blocked: Advanced ProcCall detected.") + message_admins("[key_name(usr)] attempted to edit feedback data via advanced proc-call") + log_admin("[key_name(usr)] attempted to edit feedback data via advanced proc-call") + return if(!GLOB.blackbox) return variable = sanitizeSQL(variable) @@ -364,6 +384,11 @@ proc/feedback_dec(var/variable,var/value) FV.dec(value) proc/feedback_set_details(var/variable,var/details) + if(IsAdminAdvancedProcCall()) + to_chat(usr, "Feedback edit blocked: Advanced ProcCall detected.") + message_admins("[key_name(usr)] attempted to edit feedback data via advanced proc-call") + log_admin("[key_name(usr)] attempted to edit feedback data via advanced proc-call") + return if(!GLOB.blackbox) return variable = sanitizeSQL(variable) @@ -376,6 +401,11 @@ proc/feedback_set_details(var/variable,var/details) FV.set_details(details) proc/feedback_add_details(var/variable,var/details) + if(IsAdminAdvancedProcCall()) + to_chat(usr, "Feedback edit blocked: Advanced ProcCall detected.") + message_admins("[key_name(usr)] attempted to edit feedback data via advanced proc-call") + log_admin("[key_name(usr)] attempted to edit feedback data via advanced proc-call") + return if(!GLOB.blackbox) return variable = sanitizeSQL(variable)