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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Emitter stacks fix #13539

Merged
merged 4 commits into from Jan 19, 2017
Merged

Conversation

N3X15
Copy link
Contributor

@N3X15 N3X15 commented Jan 18, 2017

Executive Summary

Fixes a stacking exploit with some machinery.

Technical information

Some fucktard decided that /obj/wrenchAnchor() should return -1 instead of 0 on failure. Guess what all other code assumes?

This also checks to see if we should anchor AFTER the do_after, in case a new dense object is added after the wait.

Changelog

馃啈

  • bugfix: Fixes machinery stacking exploit

@N3X15 N3X15 added Bug / Fix This is either a bug or a fix for a bug. Exploitable Some shitter is probably going to abuse this. Feature Loss Oh no, where'd this feature go? 100% tested I promise I tested it every possible way / HAHA HE PROMISED HE TESTED IT AND DIDN'T labels Jan 18, 2017
@ComicIronic
Copy link
Contributor

Some fucktard decided that /obj/wrenchAnchor() should return -1 instead of 0 on failure.

Because 0 is used for machines that don't anchor or deanchor. 1 is used for success in all the machine procs that I wrote in that rewrite, and -1 is used for failure. Anything using wrenchAnchor was written after that fact, and its assumptions are wrong.

@@ -564,7 +564,8 @@ Class Procs:
to_chat(user, "\The [src] has to be unwelded from the floor first.")
return -1 //state set to 2, can't do it
else
if(wrenchAnchor(user) && machine_flags && FIXED2WORK) //wrenches/unwrenches into place if possible, then updates the power and state if necessary
// wrenchAnchor returns -1 on check failures, for some reason.
if(wrenchAnchor(user) == 1 && machine_flags && FIXED2WORK) //wrenches/unwrenches into place if possible, then updates the power and state if necessary
Copy link
Contributor

Choose a reason for hiding this comment

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

How long has this check even been like this - that should be a bitwise &.

@ComicIronic
Copy link
Contributor

I'll go over the code at some point and replace the -1 and 1 occurances with success and failure defines.

@Probe1 Probe1 merged commit f403497 into vgstation-coders:Bleeding-Edge Jan 19, 2017
ihadtoregisterforthis pushed a commit to ihadtoregisterforthis/fork4 that referenced this pull request Jul 3, 2017
* Exploit fix for machine stacking.

* Fixes made during testing.

* Fix some minor indentation issues.

* honk
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
100% tested I promise I tested it every possible way / HAHA HE PROMISED HE TESTED IT AND DIDN'T Bug / Fix This is either a bug or a fix for a bug. Exploitable Some shitter is probably going to abuse this. Feature Loss Oh no, where'd this feature go?
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants