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

Can add custom ports without permission #443

Closed
3 tasks done
Fabrimat opened this issue Apr 24, 2021 · 5 comments
Closed
3 tasks done

Can add custom ports without permission #443

Fabrimat opened this issue Apr 24, 2021 · 5 comments
Assignees
Labels
core promoted security For issues that present security issues.
Projects

Comments

@Fabrimat
Copy link

Fabrimat commented Apr 24, 2021

Bug Report

System Information

  • Operating System 4.19.0-16-amd64 SMP Debian 4.19.181-1 (2021-03-19) x86_64 GNU/Linux
  • AMP version and build date v2.1.0.14, built 08/04/2021 18:11
  • Which AMP release stream you're using Mainline

I confirm:

  • that I have searched for an existing bug report for this issue.
  • that I am using the latest available version of AMP.
  • that my operating system is up-to-date.

Symptoms

  • What are you trying to do?
    Editing ports of an istance without having the proper permission
  • What are you expecting to happen?
    That it gives me an error
  • What is actually happening? ('Nothing' is not an acceptable answer!)
    The port is added without saying anything

Reproduction

  1. I created a Minecraft instance with those settings
    image
  2. Created a Test user with the following permissions (the user is not member of any group)
    image
  3. Login with a super admin and check the instance ports
    image
  4. Login with the Test user, click on the Minecraft Instance and then "Edit ports"
    image
  5. Click on the "+" in the dialog and then "Close"
  6. Check again the ports with the super admin
    image
  7. A new port has been added

On the step 5, if I do "Save changes" it will give me this error
image

@PhonicUK
Copy link
Contributor

PhonicUK commented Apr 28, 2021

Clicking + doesn't actually add the port, the network changes aren't applied until you actually hit "Save". If you fully reload the page they won't be there.

It might be incorrectly showing that they're in the list (and it shouldn't be showing the option to edit ports if you don't have permission) - but I can verify that the security model is operating correctly and the changes are not being applied when the user doesn't have permission. Indeed as it says, the API call to set the network information (the ports) requires the global Reconfigure permission for ADS.

So while it's a tad confusing that the user can even see that dialog when they won't be able to make any changes, the security model is operating as intended and there is no fault.

@Fabrimat
Copy link
Author

Fabrimat commented Apr 28, 2021

As I stated in the report, I can see the port using a different user, I forget to say that is was also with a different browser.
Also, clicking + apply immediately the changes, if i then press "Close" and then CTRL+F5 and check again the port it's still there.
Also, using the network analyzer included in the browser I can see this clicking the +:
image
image
image
And right after:
image
image

@PhonicUK PhonicUK reopened this Apr 30, 2021
@PhonicUK
Copy link
Contributor

Aah, there was a change at one point where the + and - didn't actually make the changes, indeed you're right and at the moment they do. Different API calls in that situation. Turns out, ModifyCustomFirewallRule was missing the metadata for it's permissions because originally it wasn't exposed to the web. Not a huge issue since you'd have to at least be logged in, but this will be patched immediately.

@PhonicUK PhonicUK added core promoted security For issues that present security issues. labels Apr 30, 2021
@PhonicUK PhonicUK added this to Needs triage in AMP Core via automation Apr 30, 2021
@PhonicUK PhonicUK moved this from Needs triage to High priority in AMP Core Apr 30, 2021
@PhonicUK PhonicUK moved this from High priority to Closed in AMP Core Apr 30, 2021
@PhonicUK PhonicUK self-assigned this Apr 30, 2021
@PhonicUK
Copy link
Contributor

We are assigning a CVE for this issue. Details to follow.

@PhonicUK
Copy link
Contributor

PhonicUK commented May 4, 2021

This issue was assigned CVE-2021-31926 - The issue was fixed as part of the 2.1.1.2 update.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core promoted security For issues that present security issues.
Projects
AMP Core
  
Closed
Development

No branches or pull requests

2 participants