Security flaw in admin only events

[Headcrab Zombie]

For the add-on I released two weeks ago, Environmental Control Events, I included a script to allow for events that can not be placed onto a brick by non-admins. This has worked fully from release, up until recently; exactly when, I don't know, but I came onto my server this morning, and the script wasn't working, and

someone had managed to flood the server with lava, and other environmental changes.

I tried restarting the server, and it was still broken. The registerAdminOnlyOutputEvent function was being called correctly, but nothing worked; If I deadmined myself, I could still place the events, and the /restrictedEvents command doesn't show anything. The problem persists with any sort of internet server, listen or dedicated.

However, if I start a singleplayer or lan server, using the same Blockland folder and same add-on list, and deadmin myself, everything works fine; I am unable to place the event, and the /restrictedEvents command shows every event that is unavailable to me.

I have tried running on a clean install (except for this add-on of course) and the problem still occurs

Since this add-on is currently publicly available, I am rushing myself to figure out what is going wrong. If anyone else can see anything, help would be greatly appreciated

EDIT: I just realized:it was working, it was just detecting host level which is greater than super admin. Silly me.
However, the problem still remains that a non-admin was able to bypass the checks.
This thread is now about "can you find any security loopholes in the code"

[Ad Bot]

[Mold]

%class = getWord(getField($InputEvent_TargetListfxDTSBrick_[%input],%a),1);
eval("%name = $OutputEvent_Name" @ %class @ "_" @ %output @ ";");
%reqLevel = getWord($AdminOutputEvent[%class,%name],0);

Whoops forget it
I though the local variable %name is gone after );

Anyways there is absolutely no reason to use eval there

Code: [Select]

%name = $OutputEvent_Name[%class, %output];

[Port]

%class = getWord(getField($InputEvent_TargetListfxDTSBrick_[%input],%a),1);
eval("%name = $OutputEvent_Name" @ %class @ "_" @ %output @ ";");
%reqLevel = getWord($AdminOutputEvent[%class,%name],0);

Eval isn't a security flaw unless you pass unmodified user input to it. Here it's using data from a table provided by Blockland that can only be modified if you directly can send console input already (i.e. entering code into console, cs files, chat eval, etc.), which, if you have access to, removes the need of having to go around to modify that to gain eval access anyway.

[Mold]

Eval isn't a security flaw unless you pass unmodified user input to it. Here it's using data from a table provided by Blockland that can only be modified if you directly can send console input already (i.e. entering code into console, cs files, chat eval, etc.), which, if you have access to, removes the need of having to go around to modify that to gain eval access anyway.

Fixed

[Ipquarx]

Could the events have been placed on a non-admins brick by an admin, then a non-admin used them and/or just edited them?

[Headcrab Zombie]

Anyways there is absolutely no reason to use eval there

Code: [Select]

%name = $OutputEvent_Name[%class, %output];

All I remember is trying to do this but it didn't work, so I just used eval.
I'll change it next time I release an update, but it's not really an issue to warrant an update just to it's own.


Also, my chatbot's datafile is messed up. It consists of a lot of global variables, and a lot of them are suddenly gone. I'm wondering if somehow something deleted a lot of global variables on the server, and if whatever this something was, it also deleted variables declared by the registerAdminOnlyOutputEvent function?
I have no way of testing this theory, however

Fixed

I have no idea what you fixed, or what you're trying to prove, as what Port said is correct

Could the events have been placed on a non-admins brick by an admin, then a non-admin used them and/or just edited them?

No; the events require super-admin status to place, no one other than me has super-admin, and I know I did not place them. They were on a series of JVS buttons, each one calling a different environment event

[Mold]

I have no idea what you fixed

My post

[Lugnut]

What add-ons do you have enabled?
What kinds of logging?

[Headcrab Zombie]

My post

OH.
I thought you were doing one of those 'quote someone, edit the quote in some way, and say "Fixed"' things.
My bad.

What add-ons do you have enabled?

http://pastebin.com/jLE1PVYv
Removed the entries for disabled add-ons so you don't have to look through those

What kinds of logging?

Nothing other than default console log, which I didn't save before restarting BL

[Space Guy]

%output in the eval is still a user input.

[Port]

%output in the eval is still a user input.

Ah. I was looking directly at the code Mold posted and not the full code of the add-on so I was assuming %output was defined elsewhere.

[Headcrab Zombie]

%output in the eval is still a user input.

Well, I fixed that, and I'll get an update out soon.
I still feel like I'm still missing something though.
I just don't understand why whoever did it would use it for nothing more than to give themselves admin to place the event

This is who did it, if anyone's heard of him

[Lugnut]

>uck you man im the alex
frightening character!

how did you figure out it was him without logging?

[Headcrab Zombie]

I came on early this morning and someone told me about it and showed me the bricks with the events on them

[TripNick]

After some detailed research about that person, I think I've discovered their name is "Alex".