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

Rewrite SendTileRrect handling #2936

Merged
merged 13 commits into from
Aug 15, 2023
Merged

Rewrite SendTileRrect handling #2936

merged 13 commits into from
Aug 15, 2023

Conversation

punchready
Copy link
Contributor

@punchready punchready commented Apr 4, 2023

The old handler had a pretty significant flaw allowing attackers to basically bypass most checks, so I went to rewrite it completely.
All categories of checks have been tested successfully, but I did not perform exhaustive checks for every potential action due to lack of time, so if needed someone could do that based on the list of operations in the code.
While writing this it also became apparent again that tShock does not have a way to restrict wall placement like it does with tiles, which would matter during biome conversion handling.

@punchready punchready changed the title Rewrite STR handling Rewrite SendTileRrect handling Apr 5, 2023
Copy link
Member

@QuiCM QuiCM left a comment

Choose a reason for hiding this comment

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

Rewriting Terraria code to protect against its own exploits makes me cry, but it is what it is.

Only real change I'd like is the removal of the existing SendTileRectHandler, and rename this to take its place

@punchready
Copy link
Contributor Author

Yeah I wanted to avoid making the diff unreadable, which at the very least requires a different file name which would also suck. I guess I can change the class name after everyone has had a chance to check it out while the diff is still clean.

@punchready punchready requested a review from QuiCM May 9, 2023 10:46
QuiCM
QuiCM previously approved these changes Jun 6, 2023
@QuiCM
Copy link
Member

QuiCM commented Jun 6, 2023

Couple minor Qs, but overall I'm happy to trust that this works

TShockAPI/Handlers/SendTileRectHandler.cs Show resolved Hide resolved
TShockAPI/Handlers/SendTileRectHandler.cs Show resolved Hide resolved
TShockAPI/Handlers/SendTileRectHandler.cs Outdated Show resolved Hide resolved
Arthri
Arthri previously approved these changes Jun 6, 2023
@punchready punchready requested a review from QuiCM June 7, 2023 04:36
@QuiCM QuiCM merged commit 2d507cc into Pryaxis:general-devel Aug 15, 2023
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants