diff --git a/code/__HELPERS/sanitize_values.dm b/code/__HELPERS/sanitize_values.dm index 5020693a499..6777f43ff91 100644 --- a/code/__HELPERS/sanitize_values.dm +++ b/code/__HELPERS/sanitize_values.dm @@ -50,18 +50,3 @@ HSL[3] = min(HSL[3],0.4) var/list/RGB = hsl2rgb(arglist(HSL)) return "#[num2hex(RGB[1],2)][num2hex(RGB[2],2)][num2hex(RGB[3],2)]" - - -// Sanitize inputs to avoid SQL injection attacks -/proc/sql_sanitize_text(var/text) - text = replacetext(text, "'", "''") - text = replacetext(text, ";", "") - text = replacetext(text, "&", "") - return text - -// Calls the above proc on each entry of a list to ensure its entries are clean -/proc/sql_sanitize_text_list(var/list/l) - var/list/new_list = l.Copy() - for(var/text in new_list) - sql_sanitize_text(text) - return new_list \ No newline at end of file diff --git a/code/__HELPERS/text.dm b/code/__HELPERS/text.dm index e56ed3a4310..40eaa9007a1 100644 --- a/code/__HELPERS/text.dm +++ b/code/__HELPERS/text.dm @@ -15,6 +15,10 @@ // Run all strings to be used in an SQL query through this proc first to properly escape out injection attempts. /proc/sanitizeSQL(var/t as text) + if(isnull(t)) + return null + if(!istext(t)) + t = "[t]" // Just quietly assume any non-texts are supposed to be text var/sqltext = dbcon.Quote(t); return copytext(sqltext, 2, lentext(sqltext));//Quote() adds quotes around input, we already do that diff --git a/code/modules/admin/DB ban/functions.dm b/code/modules/admin/DB ban/functions.dm index 3027864ace6..c22a019addb 100644 --- a/code/modules/admin/DB ban/functions.dm +++ b/code/modules/admin/DB ban/functions.dm @@ -113,7 +113,7 @@ datum/admins/proc/DB_ban_record(var/bantype, var/mob/banned_mob, var/duration = else adminwho += ", [C]" - reason = sql_sanitize_text(reason) + reason = sanitizeSQL(reason) if(maxadminbancheck) var/DBQuery/adm_query = dbcon.NewQuery("SELECT count(id) AS num FROM [format_table_name("ban")] WHERE (a_ckey = '[a_ckey]') AND (bantype = 'ADMIN_PERMABAN' OR (bantype = 'ADMIN_TEMPBAN' AND expiration_time > Now())) AND isnull(unbanned)") @@ -244,14 +244,14 @@ datum/admins/proc/DB_ban_edit(var/banid = null, var/param = null) to_chat(usr, "Invalid ban id. Contact the database admin") return - reason = sql_sanitize_text(reason) + reason = sanitizeSQL(reason) var/value switch(param) if("reason") if(!value) value = input("Insert the new reason for [pckey]'s ban", "New Reason", "[reason]", null) as null|text - value = sql_sanitize_text(value) + value = sanitizeSQL(value) if(!value) to_chat(usr, "Cancelled") return @@ -417,8 +417,8 @@ datum/admins/proc/DB_ban_unban_by_id(var/id) adminckey = ckey(adminckey) playerckey = ckey(playerckey) - playerip = sql_sanitize_text(playerip) - playercid = sql_sanitize_text(playercid) + playerip = sanitizeSQL(playerip) + playercid = sanitizeSQL(playercid) if(adminckey || playerckey || playerip || playercid || dbbantype) diff --git a/code/modules/admin/banappearance.dm b/code/modules/admin/banappearance.dm index b039f6bd634..06deaa1c461 100644 --- a/code/modules/admin/banappearance.dm +++ b/code/modules/admin/banappearance.dm @@ -92,18 +92,3 @@ DEBUG appearance_savebanfile() return 1 return 0 - -/* -proc/DB_ban_isappearancebanned(var/playerckey) - establish_db_connection() - if(!dbcon.IsConnected()) - return - - var/sqlplayerckey = sql_sanitize_text(ckey(playerckey)) - - var/DBQuery/query = dbcon.NewQuery("SELECT id FROM [format_table_name("ban")] WHERE CKEY = '[sqlplayerckey]' AND ((bantype = 'APPEARANCE_BAN') OR (bantype = 'APPEARANCE_TEMPBAN' AND expiration_time > Now())) AND unbanned != 1") - query.Execute() - while(query.NextRow()) - return 1 - return 0 -*/ \ No newline at end of file diff --git a/code/modules/client/client procs.dm b/code/modules/client/client procs.dm index 7a87264d815..ed4e648e6d0 100644 --- a/code/modules/client/client procs.dm +++ b/code/modules/client/client procs.dm @@ -368,9 +368,7 @@ if(!dbcon.IsConnected()) return - var/sql_ckey = sql_sanitize_text(src.ckey) - - var/DBQuery/query = dbcon.NewQuery("SELECT id, datediff(Now(),firstseen) as age FROM [format_table_name("player")] WHERE ckey = '[sql_ckey]'") + var/DBQuery/query = dbcon.NewQuery("SELECT id, datediff(Now(),firstseen) as age FROM [format_table_name("player")] WHERE ckey = '[ckey]'") query.Execute() var/sql_id = 0 player_age = 0 // New players won't have an entry so knowing we have a connection we set this to zero to be updated if their is a record. @@ -397,7 +395,7 @@ if(related_accounts_cid.len) log_access("Alts: [key_name(src)]:[jointext(related_accounts_cid, " - ")]") - var/watchreason = check_watchlist(sql_ckey) + var/watchreason = check_watchlist(ckey) if(watchreason) message_admins("Notice: [key_name_admin(src)] is on the watchlist and has just connected - Reason: [watchreason]") send2adminirc("Watchlist - [key_name(src)] is on the watchlist and has just connected - Reason: [watchreason]") @@ -413,9 +411,9 @@ if(src.holder) admin_rank = src.holder.rank - var/sql_ip = sql_sanitize_text(src.address) - var/sql_computerid = sql_sanitize_text(src.computer_id) - var/sql_admin_rank = sql_sanitize_text(admin_rank) + var/sql_ip = sanitizeSQL(address) + var/sql_computerid = sanitizeSQL(computer_id) + var/sql_admin_rank = sanitizeSQL(admin_rank) if(sql_id) @@ -424,12 +422,12 @@ query_update.Execute() else //New player!! Need to insert all the stuff - var/DBQuery/query_insert = dbcon.NewQuery("INSERT INTO [format_table_name("player")] (id, ckey, firstseen, lastseen, ip, computerid, lastadminrank) VALUES (null, '[sql_ckey]', Now(), Now(), '[sql_ip]', '[sql_computerid]', '[sql_admin_rank]')") + var/DBQuery/query_insert = dbcon.NewQuery("INSERT INTO [format_table_name("player")] (id, ckey, firstseen, lastseen, ip, computerid, lastadminrank) VALUES (null, '[ckey]', Now(), Now(), '[sql_ip]', '[sql_computerid]', '[sql_admin_rank]')") query_insert.Execute() //Logging player access var/serverip = "[world.internet_address]:[world.port]" - var/DBQuery/query_accesslog = dbcon.NewQuery("INSERT INTO `[format_table_name("connection_log")]`(`id`,`datetime`,`serverip`,`ckey`,`ip`,`computerid`) VALUES(null,Now(),'[serverip]','[sql_ckey]','[sql_ip]','[sql_computerid]');") + var/DBQuery/query_accesslog = dbcon.NewQuery("INSERT INTO `[format_table_name("connection_log")]`(`id`,`datetime`,`serverip`,`ckey`,`ip`,`computerid`) VALUES(null,Now(),'[serverip]','[ckey]','[sql_ip]','[sql_computerid]');") query_accesslog.Execute() diff --git a/code/modules/client/preference/preferences.dm b/code/modules/client/preference/preferences.dm index 635169bf89b..c39a34d969a 100644 --- a/code/modules/client/preference/preferences.dm +++ b/code/modules/client/preference/preferences.dm @@ -1755,12 +1755,7 @@ var/global/list/special_role_times = list( //minimum age (in days) for accounts if("be_special") var/r = href_list["role"] - if(!(r in special_roles)) - var/cleaned_r = sql_sanitize_text(r) - if(r != cleaned_r) // up to no good - message_admins("[user] attempted an href exploit! (This could have possibly lead to a \"Bobby Tables\" exploit, so they're probably up to no good). String: [r] ID: [last_id] IP: [last_ip]") - to_chat(user, "Stop right there, criminal scum") - else + if(r in special_roles) be_special ^= r if("name") diff --git a/code/modules/client/preference/preferences_mysql.dm b/code/modules/client/preference/preferences_mysql.dm index f72ac076dff..dbdc97d11e1 100644 --- a/code/modules/client/preference/preferences_mysql.dm +++ b/code/modules/client/preference/preferences_mysql.dm @@ -70,7 +70,7 @@ UI_style='[UI_style]', UI_style_color='[UI_style_color]', UI_style_alpha='[UI_style_alpha]', - be_role='[list2params(sql_sanitize_text_list(be_special))]', + be_role='[sanitizeSQL(list2params(be_special))]', default_slot='[default_slot]', toggles='[toggles]', sound='[sound]', @@ -227,6 +227,15 @@ med_record = query.item[49] sec_record = query.item[50] gen_record = query.item[51] + // Apparently, the preceding vars weren't always encoded properly... + if(findtext(flavor_text, "<")) // ... so let's clumsily check for tags! + flavor_text = html_encode(flavor_text) + if(findtext(med_record, "<")) + med_record = html_encode(med_record) + if(findtext(sec_record, "<")) + sec_record = html_encode(sec_record) + if(findtext(gen_record, "<")) + gen_record = html_encode(gen_record) disabilities = text2num(query.item[52]) player_alt_titles = params2list(query.item[53]) organ_data = params2list(query.item[54]) @@ -322,13 +331,13 @@ firstquery.Execute() while(firstquery.NextRow()) if(text2num(firstquery.item[1]) == default_slot) - var/DBQuery/query = dbcon.NewQuery({"UPDATE [format_table_name("characters")] SET OOC_Notes='[sql_sanitize_text(metadata)]', - real_name='[sql_sanitize_text(real_name)]', + var/DBQuery/query = dbcon.NewQuery({"UPDATE [format_table_name("characters")] SET OOC_Notes='[sanitizeSQL(metadata)]', + real_name='[sanitizeSQL(real_name)]', name_is_always_random='[be_random_name]', gender='[gender]', age='[age]', - species='[sql_sanitize_text(species)]', - language='[sql_sanitize_text(language)]', + species='[sanitizeSQL(species)]', + language='[sanitizeSQL(language)]', hair_red='[r_hair]', hair_green='[g_hair]', hair_blue='[b_hair]', @@ -345,10 +354,10 @@ head_accessory_red='[r_headacc]', head_accessory_green='[g_headacc]', head_accessory_blue='[b_headacc]', - hair_style_name='[sql_sanitize_text(h_style)]', - facial_style_name='[sql_sanitize_text(f_style)]', - marking_style_name='[sql_sanitize_text(m_style)]', - head_accessory_style_name='[sql_sanitize_text(ha_style)]', + hair_style_name='[sanitizeSQL(h_style)]', + facial_style_name='[sanitizeSQL(f_style)]', + marking_style_name='[sanitizeSQL(m_style)]', + head_accessory_style_name='[sanitizeSQL(ha_style)]', eyes_red='[r_eyes]', eyes_green='[g_eyes]', eyes_blue='[b_eyes]', @@ -369,10 +378,10 @@ job_karma_high='[job_karma_high]', job_karma_med='[job_karma_med]', job_karma_low='[job_karma_low]', - flavor_text='[sql_sanitize_text(html_decode(flavor_text))]', - med_record='[sql_sanitize_text(html_decode(med_record))]', - sec_record='[sql_sanitize_text(html_decode(sec_record))]', - gen_record='[sql_sanitize_text(html_decode(gen_record))]', + flavor_text='[sanitizeSQL(flavor_text)]', + med_record='[sanitizeSQL(med_record)]', + sec_record='[sanitizeSQL(sec_record)]', + gen_record='[sanitizeSQL(gen_record)]', player_alt_titles='[playertitlelist]', disabilities='[disabilities]', organ_data='[organlist]', @@ -401,7 +410,10 @@ skin_tone, skin_red, skin_green, skin_blue, markings_red, markings_green, markings_blue, head_accessory_red, head_accessory_green, head_accessory_blue, - hair_style_name, facial_style_name, marking_style_name, head_accessory_style_name, + hair_style_name, + facial_style_name, + marking_style_name, + head_accessory_style_name, eyes_red, eyes_green, eyes_blue, underwear, undershirt, backbag, b_type, alternate_option, @@ -409,20 +421,26 @@ job_medsci_high, job_medsci_med, job_medsci_low, job_engsec_high, job_engsec_med, job_engsec_low, job_karma_high, job_karma_med, job_karma_low, - flavor_text, med_record, sec_record, gen_record, + flavor_text, + med_record, + sec_record, + gen_record, player_alt_titles, disabilities, organ_data, rlimb_data, nanotrasen_relation, speciesprefs, socks, body_accessory, gear) VALUES - ('[C.ckey]', '[default_slot]', '[sql_sanitize_text(metadata)]', '[sql_sanitize_text(real_name)]', '[be_random_name]','[gender]', - '[age]', '[sql_sanitize_text(species)]', '[sql_sanitize_text(language)]', + ('[C.ckey]', '[default_slot]', '[sanitizeSQL(metadata)]', '[sanitizeSQL(real_name)]', '[be_random_name]','[gender]', + '[age]', '[sanitizeSQL(species)]', '[sanitizeSQL(language)]', '[r_hair]', '[g_hair]', '[b_hair]', '[r_facial]', '[g_facial]', '[b_facial]', '[s_tone]', '[r_skin]', '[g_skin]', '[b_skin]', '[r_markings]', '[g_markings]', '[b_markings]', '[r_headacc]', '[g_headacc]', '[b_headacc]', - '[sql_sanitize_text(h_style)]', '[sql_sanitize_text(f_style)]', '[sql_sanitize_text(m_style)]', '[sql_sanitize_text(ha_style)]', + '[sanitizeSQL(h_style)]', + '[sanitizeSQL(f_style)]', + '[sanitizeSQL(m_style)]', + '[sanitizeSQL(ha_style)]', '[r_eyes]', '[g_eyes]', '[b_eyes]', '[underwear]', '[undershirt]', '[backbag]', '[b_type]', '[alternate_option]', @@ -430,7 +448,10 @@ '[job_medsci_high]', '[job_medsci_med]', '[job_medsci_low]', '[job_engsec_high]', '[job_engsec_med]', '[job_engsec_low]', '[job_karma_high]', '[job_karma_med]', '[job_karma_low]', - '[sql_sanitize_text(html_encode(flavor_text))]', '[sql_sanitize_text(html_encode(med_record))]', '[sql_sanitize_text(html_encode(sec_record))]', '[sql_sanitize_text(html_encode(gen_record))]', + '[sanitizeSQL(flavor_text)]', + '[sanitizeSQL(med_record)]', + '[sanitizeSQL(sec_record)]', + '[sanitizeSQL(gen_record)]', '[playertitlelist]', '[disabilities]', '[organlist]', '[rlimblist]', '[nanotrasen_relation]', '[speciesprefs]', '[socks]', '[body_accessory]', '[gearlist]') diff --git a/code/modules/customitems/item_spawning.dm b/code/modules/customitems/item_spawning.dm index 345bf6e164e..11df4b13cf9 100644 --- a/code/modules/customitems/item_spawning.dm +++ b/code/modules/customitems/item_spawning.dm @@ -3,7 +3,7 @@ return // Grab the info we want. - var/DBQuery/query = dbcon.NewQuery("SELECT cuiPath, cuiPropAdjust, cuiJobMask, cuiDescription, cuiItemName FROM [format_table_name("customuseritems")] WHERE cuiCKey='[M.ckey]' AND (cuiRealName='[M.real_name]' OR cuiRealName='*')") + var/DBQuery/query = dbcon.NewQuery("SELECT cuiPath, cuiPropAdjust, cuiJobMask, cuiDescription, cuiItemName FROM [format_table_name("customuseritems")] WHERE cuiCKey='[M.ckey]' AND (cuiRealName='[sanitizeSQL(M.real_name)]' OR cuiRealName='*')") query.Execute() while(query.NextRow()) diff --git a/code/modules/mob/living/silicon/silicon.dm b/code/modules/mob/living/silicon/silicon.dm index 5dd9ea3b09e..65c5c54b57b 100644 --- a/code/modules/mob/living/silicon/silicon.dm +++ b/code/modules/mob/living/silicon/silicon.dm @@ -242,7 +242,7 @@ set desc = "Sets an extended description of your character's features." set category = "IC" - flavor_text = sanitize(input(usr, "Please enter your new flavour text.", "Flavour text", null) as text) + update_flavor_text() /mob/living/silicon/binarycheck() return 1 diff --git a/code/modules/research/message_server.dm b/code/modules/research/message_server.dm index 651f278cdd8..815f6d1fe58 100644 --- a/code/modules/research/message_server.dm +++ b/code/modules/research/message_server.dm @@ -315,7 +315,7 @@ var/obj/machinery/blackbox_recorder/blackbox proc/feedback_set(var/variable,var/value) if(!blackbox) return - variable = sql_sanitize_text(variable) + variable = sanitizeSQL(variable) var/datum/feedback_variable/FV = blackbox.find_feedback_datum(variable) @@ -326,7 +326,7 @@ proc/feedback_set(var/variable,var/value) proc/feedback_inc(var/variable,var/value) if(!blackbox) return - variable = sql_sanitize_text(variable) + variable = sanitizeSQL(variable) var/datum/feedback_variable/FV = blackbox.find_feedback_datum(variable) @@ -337,7 +337,7 @@ proc/feedback_inc(var/variable,var/value) proc/feedback_dec(var/variable,var/value) if(!blackbox) return - variable = sql_sanitize_text(variable) + variable = sanitizeSQL(variable) var/datum/feedback_variable/FV = blackbox.find_feedback_datum(variable) @@ -348,8 +348,8 @@ proc/feedback_dec(var/variable,var/value) proc/feedback_set_details(var/variable,var/details) if(!blackbox) return - variable = sql_sanitize_text(variable) - details = sql_sanitize_text(details) + variable = sanitizeSQL(variable) + details = sanitizeSQL(details) var/datum/feedback_variable/FV = blackbox.find_feedback_datum(variable) @@ -360,8 +360,8 @@ proc/feedback_set_details(var/variable,var/details) proc/feedback_add_details(var/variable,var/details) if(!blackbox) return - variable = sql_sanitize_text(variable) - details = sql_sanitize_text(details) + variable = sanitizeSQL(variable) + details = sanitizeSQL(details) var/datum/feedback_variable/FV = blackbox.find_feedback_datum(variable)