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

Execute simple check before enable BungeeCord hook. #2572

Merged
merged 1 commit into from
Jul 6, 2022
Merged

Execute simple check before enable BungeeCord hook. #2572

merged 1 commit into from
Jul 6, 2022

Conversation

Ghost-chu
Copy link
Contributor

@Ghost-chu Ghost-chu commented Jul 4, 2022

This PR added a simple check that check spigot.yml -> settings.bungeecord status by using AuthMe built-in method bukkitService.isBungeeCordConfiguredForSpigot() and disable hook if it enabled and not behind an BungeeCord proxy.

Register plugin message channel without BungeeCord proxy will allow attacker send fake login payload to treat AuthMe login with Plugin Message for him and bypass the user login.

Fixed:
#2571
#2559

@games647 games647 added the security Pull requests that address a security vulnerability label Jul 4, 2022
Copy link
Member

@games647 games647 left a comment

Choose a reason for hiding this comment

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

Good idea, but maybe we should also add an entry to the SettingsWarner, so that we don't disable functionality for the user without telling them the reason. There might be cases where people actually forget to turn on this BungeeCord setting (the spigot.yml one).

@Ghost-chu Ghost-chu requested a review from games647 July 4, 2022 15:12
Copy link
Member

@games647 games647 left a comment

Choose a reason for hiding this comment

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

!LGTM

"!LGTM" (Looks good to me) in pixel GitHub commit history where pixel represents a day of the year with a high commit count

Although this potentially break Velocity support. However we don't support Velocity officially, so I think it's fine.

@games647
Copy link
Member

games647 commented Jul 6, 2022

Could you squash them into a single commit and then it's good to go.

This commit added a simple check that check spigot.yml -> settings.bungeecord status by using AuthMe built-in method bukkitService.isBungeeCordConfiguredForSpigot() and disable hook if it enabled and not behind an BungeeCord proxy.

Register plugin message channel without BungeeCord proxy will allow attacker send fake login payload to treat AuthMe login with Plugin Message for him and bypass the user login.

This commit also updated SettingsWarner for new behavior.
@Ghost-chu
Copy link
Contributor Author

Ghost-chu commented Jul 6, 2022

@games647 It's squashed to one single commit now ;)

@games647 games647 merged commit 32d92e1 into AuthMe:master Jul 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
security Pull requests that address a security vulnerability Topic: bungeecord
Development

Successfully merging this pull request may close these issues.

2 participants