Merge pull request #5484 from tkdrg/permissionabuse

Improves Permissions Panel security

code/modules/admin/admin_ranks.dm

@@ -222,6 +222,10 @@ var/list/admin_ranks = list()								//list of all admin_rank datums
 		if("remove")
 			if(alert("Are you sure you want to remove [adm_ckey]?","Message","Yes","Cancel") == "Yes")
 				if(!D)	return
+				if(!check_if_greater_rights_than_holder(D))
+					message_admins("[key_name_admin(usr)] attempted to remove [adm_ckey] from the admins list without sufficient rights.")
+					log_admin("[key_name(usr)] attempted to remove [adm_ckey] from the admins list without sufficient rights.")
+					return
 				admin_datums -= adm_ckey
 				D.disassociate()
 
@@ -244,6 +248,12 @@ var/list/admin_ranks = list()								//list of all admin_rank datums
 					new_rank = ckeyEx(input("Please input a new rank", "New custom rank", null, null) as null|text)
 					if(!new_rank)	return
 
+			if(D)
+				if(!check_if_greater_rights_than_holder(D))
+					message_admins("[key_name_admin(usr)] attempted to change the rank of [adm_ckey] to [new_rank] without sufficient rights.")
+					log_admin("[key_name(usr)] attempted to change the rank of [adm_ckey] to [new_rank] without sufficient rights.")
+					return
+
 			R = rank_names[new_rank]
 			if(!R)	//rank with that name doesn't exist yet - make it
 				if(D)	R = new(new_rank, D.rank.rights, D.rank.adds, D.rank.subs)	//duplicate our previous admin_rank but with a new name
@@ -269,6 +279,11 @@ var/list/admin_ranks = list()								//list of all admin_rank datums
 			var/keyword = input("Input permission keyword (one at a time):\ne.g. +BAN or -FUN or +/client/proc/someverb", "Permission toggle", null, null) as null|text
 			if(!keyword)	return
 
+			if(!check_if_greater_rights_than_holder(D))
+				message_admins("[key_name_admin(usr)] attempted to give [adm_ckey] the keyword [keyword] without sufficient rights.")
+				log_admin("[key_name(usr)] attempted to give [adm_ckey] the keyword [keyword] without sufficient rights.")
+				return
+
 			D.disassociate()
 
 			if(!findtext(D.rank.name, "([adm_ckey])"))	//not a modified subrank, need to duplicate the admin_rank datum to prevent modifying others too

code/modules/admin/holder2.dm

@@ -40,6 +40,16 @@ var/list/admin_datums = list()
 		owner.holder = null
 		owner = null
 
+/datum/admins/proc/check_if_greater_rights_than_holder(datum/admins/other)
+	if(!other)
+		return 1 //they have no rights
+	if(rank.rights == 65535)
+		return 1 //we have all the rights
+	if(rank.rights != other.rank.rights)
+		if( (rank.rights & other.rank.rights) == other.rank.rights )
+			return 1 //we have all the rights they have and more
+	return 0
+
 /*
 checks if usr is an admin with at least ONE of the flags in rights_required. (Note, they don't need all the flags)
 if rights_required == 0, then it simply checks if they are an admin.
@@ -68,13 +78,9 @@ you will have to do something like if(client.rights & R_ADMIN) yourself.
 		if(usr.client.holder)
 			if(!other || !other.holder)
 				return 1
-			if(usr.client.holder.rank.rights != other.holder.rank.rights)	//Check values smaller than 65536
-				if( (usr.client.holder.rank.rights & other.holder.rank.rights) == other.holder.rank.rights )
-					return 1	//we have all the rights they have and more
-		usr << "<font color='red'>Error: Cannot proceed. They have greater or equal rights to us.</font>"
+			return usr.client.holder.check_if_greater_rights_than_holder(other.holder)
 	return 0
 
-
 /client/proc/deadmin()
 	admin_datums -= ckey
 	if(holder)

code/modules/admin/topic.dm

@@ -784,6 +784,7 @@
 		var/mob/M = locate(href_list["boot2"])
 		if (ismob(M))
 			if(!check_if_greater_rights_than(M.client))
+				usr << "<span class='danger'>Error: They have more rights than you do.</span>"
 				return
 			M << "<span class='danger'>You have been kicked from the server.</span>"
 			log_admin("[key_name(usr)] booted [key_name(M)].")