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(server/impl): initConnect addition overflow #2330

Merged
merged 1 commit into from
Jan 8, 2024

Conversation

blattersturm
Copy link
Contributor

@blattersturm blattersturm commented Jan 7, 2024

Invalid ticket lengths could lead to an out-of-bounds read, crashing the server. This is relatively easy to exploit as well (just change the field in an existing valid cfxTicket to FF FF FF FF, and send it on to a server you want to crash) and therefore a bit risky - I've been trying to report this privately for ages (see the Sep 6, 2023 commit date!), but it seems to have slipped through the cracks and not been acted on at all for the past four months.

Goal of this PR

Make the server not crash when sending an invalid ticket.

How is this PR achieving the goal

Using larger types.

This PR applies to the following area(s)

Server

Successfully tested on

Platforms: Windows. Should work on Linux as well.

Checklist

  • Code compiles and has been tested successfully.
  • Code explains itself well and/or is documented.
  • My commit message explains what the changes do and what they are for.
  • No extra compilation warnings are added by these changes.

Invalid ticket lengths could lead to an out-of-bounds read.
@z3t4s
Copy link
Contributor

z3t4s commented Jan 7, 2024

It took me 20 minutes to successfully crash my test server, and most of that was compiling...
Deferral (Queue systems, Allow lists) do not help you. Update to the newest artifacts as soon as they release.

If you're a high risk target with good infrastructure you might want to filter traffic arriving on POST /client for bad tickets for now.

Demo: https://streamable.com/2gxesr

Semi OT: It would be good to see some movement on a few other, even more dangerous issues that have been reported to the appropriate parties in private

@Vinipux322
Copy link
Contributor

Hey bubble, long time no see, buddy, how you doing?

Demo: https://www.youtube.com/watch?v=v85xbzENTYQ

Demo

@nihonium-cfx nihonium-cfx merged commit a654bcc into citizenfx:master Jan 8, 2024
2 of 10 checks passed
@nihonium-cfx
Copy link
Contributor

For the future, we encourage everyone to submit exploit reports privately through [email protected] or by contacting an element on discord directly.

@citizenfx citizenfx locked as resolved and limited conversation to collaborators Jan 8, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants