Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

exploit fix, electric boogaloo : canister edition #5068

Merged
merged 1 commit into from
May 26, 2014
Merged

exploit fix, electric boogaloo : canister edition #5068

merged 1 commit into from
May 26, 2014

Conversation

Walter0o
Copy link
Contributor

024a0ba removed the exploit protection of canisters without replacement, this commit fixes that.

without this, anybody can open/close valves and all other forms of messing with any canister from anywhere.

ALWAYS keep in mind, NanoUI will update the fancy buttons CLIENTSIDE, that does NOT protect against exploits !

the actual buttonpush can still be spoofed to the server even if NanoUI disabled/hid/greyed-out/removed the button !

024a0ba removed the exploit protection of canisters without replacement, this commit fixes that.

without this, anybody can open/close valves and all other forms of messing with any canister from anywhere.

ALWAYS keep in mind, NanoUI will update the fancy buttons CLIENTSIDE, that does NOT protect against exploits !

the actual buttonpush can still be spoofed to the server even if NanoUI disabled/hid/greyed-out/removed the button !
@Walter0o
Copy link
Contributor Author

the same goes for non-NanoUI, standard html buttons/windows too, btw

@MistakeNot4892
Copy link
Contributor

👍

comma added a commit that referenced this pull request May 26, 2014
exploit fix, electric boogaloo : canister edition
@comma comma merged commit a43249b into Baystation12:master May 26, 2014
if (!istype(src.loc, /turf))
return 0


if(!usr.canmove || usr.stat || usr.restrained() || !in_range(loc, usr)) // exploit protection -walter0o
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From what I understand, Adjacent() is preferable to in_range() for mobs interacting with objects since it prevents using things through glass panels.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, ClickOn() already handles adjacency, so Adjacent() may not be necessary, except for the weird case where you open the canister dialog and then move so that there is a glass panel between you and the canister. Probably not worth worrying about.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see my comment below, i thought i posted it in here.

@Walter0o
Copy link
Contributor Author

@mwerezak , at the risk of coming off as a dick, did you not read the commit description i gave ?

HTML and NanoUI windows have literally NO exploit protection potentional whatsoever, they are clientside, them hiding and adding and removing and greying out buttons is eye-candy only !

this means if you dont have the object's menu and you are laying dead on the other side of the map on another z-level,
you can STILL spoof the buttonclick to the server anyway.

for example :
you being a mangled dead limbless corpse stuffed in a welded locker floating through space does not restrict you in any way to spoof the buttonpush to the locked turret control of the AI core,
telling it to switch to lethal lasers.
only the turret control's exploit checks and how it reacts to incoming commands will prevent it from executing your spoofed commands, nothing else.

you are right to argue Adjacent() might be better than in_range(), but not via ClickOn().
ClickOn() even after its adjancy checks will or will not eventually call the HTML or NanoUI window handlers, which means ClickOn() will do NOTHING to prevent exploits.

this commits seeks to restore a basic protection against exploiters being able to open canisters without being restricted by their distance to the object, death, handcuffs or whatever, so feel free to add more checks to those i restored or replace them with better versions, but there MUST be some in the Topic proc.

again, ANY windows and buttons, HTML or NanoUI, have ZERO exploit protection potentional, do not rely on them when you deal with exploit prevention, you HAVE to do that in the Topic proc NO MATTER WHAT !

almost all exploits i ever saw stem from devs not being regorous with this and failing to doublecheck other devs work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants