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

Fix an HTML injection exploit #1210

Merged
merged 3 commits into from Jul 13, 2022

Conversation

brandonsturgeon
Copy link
Contributor

@brandonsturgeon brandonsturgeon commented Jul 13, 2022

Because the URL for this part is injected into an HTML blob, attackers can manually send a malicious part that can run arbitrary javascript code on any client that renders their PAC.

My fix updates the pac.FixUrl method to strip out exploitative HTML characters (that aren't valid in URLs anyway), and ensures that the size parameter is a number before concatenating it into the HTML blob.

This has been tested on our server and works as expected with no noticeable side effects.

Big thanks to @plally for helping me quickly investigate this.

@thegrb93 thegrb93 merged commit 67d9349 into CapsAdmin:master Jul 13, 2022
@brandonsturgeon brandonsturgeon deleted the fix/improve-fixurl branch July 13, 2022 05:55
CapsAdmin pushed a commit that referenced this pull request Jul 13, 2022
* Improve FixURL

* Ensure number

* Simplify
CapsAdmin pushed a commit that referenced this pull request Jul 13, 2022
* Improve FixURL

* Ensure number

* Simplify
@Fasteroid
Copy link

Can we please merge this on the develop branch already

thegrb93 added a commit that referenced this pull request Oct 28, 2022
* make CanTool check more robust

* Fix an HTML injection exploit (#1210)

* Improve FixURL

* Ensure number

* Simplify

* Update CONTRIBUTING.md

* Prevent players from mutating entities if they're pac banned

Co-authored-by: CapsAdmin <eliashogstvedt@gmail.com>
Co-authored-by: Brandon Sturgeon <brandon@brandonsturgeon.com>
Co-authored-by: Techbot121 <techbot121@gmail.com>
Co-authored-by: Redox <mail@redox.to>
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

3 participants