U4-2021 - Ran a penetration test and was able to do this - how can it be eliminated

Created by Anthony jones 28 Mar 2013, 18:15:37 Updated by Sebastiaan Janssen 19 Apr 2013, 07:01:59

Relates to: U4-529

Relates to: U4-1373

Relates to: U4-2027

Observation Vertical privilege escalation Description Vertical privilege escalation is observed in the application. In the current application scenario, lower privileged user "testeditor" has access to only "Content" and "Media" modules and does not have access to higher privileged admin modules such as "Settings", "Developer", "Users" and "Members". Admin user can create new users by accessing the "Users" module on the Umbraco page. However, it is possible to create users using the lower privileged "testeditor" user by requesting the "create.aspx" file directly. The URL is: http://156.109.215.144/umbraco/create.aspx?nodeId=-1&nodeType=users&nodeName=Users&rnd=3 5.6&rndo=39.1 The parameters to be sent along this request are: __EVENTTARGET=&__EVENTARGUMENT=&__VIEWSTATE=%2FwEPDwUKMjA1MDg3OTkzOQ9 kFgJmD2QWAgIDEGRkFgICAw9kFgICAQ9kFgJmD2QWAgIDDw8WAh4EVGV4dAUGQ3JlYXRlZGR kbaP1imhzmea%2BvxspiXS9X%2BfKEqmk%2F%2B0Ad31Op1LPN%2Bo%3D&nodeType=&ctl00%2 4body%24ctl00%24rename=testusertest&ctl00%24body%24ctl00%24Textbox1=&ctl00%24body%2 4ctl00%24sbmt=Create Where the ctl00%24body%24ctl00%24rename parameter is the username of the user to be created. Testing team successfully created a user called "testusertest" using "testeditor" user. Impact An authenticated attacker can leverage this vulnerability and create users for the application thereby impacting the integrity of the application.

Comments

Sebastiaan Janssen 29 Mar 2013, 11:24:04

Thanks for the report, while we evaluate and fix this issue I'm setting it to be readable by Umbraco HQ only so to prevent exploits.


Anthony jones 03 Apr 2013, 15:32:26

Sebastian. We have a client for which we need to resolve this. Would it be possible to turn this around quickly or what other options do I have? Also for the reflected XSS issue.


Sebastiaan Janssen 03 Apr 2013, 15:37:47

We're working on this, it requires changes that will break a lot of existing packages. We're thinking of a better way to fix this but it's not something we can crack in 5 minutes. Thanks again for the report, we're doing the best we can to provide a solution but it will take some time, thank you for your patience.


Anthony jones 03 Apr 2013, 15:40:40

Thanks. I figured as much. Do you have any idea - week, month? Just trying to manage my client. Maybe i can put someone on my team on it?


Sebastiaan Janssen 03 Apr 2013, 16:10:26

I'll have an update by the end of the week, we do also want to move this forward as fast as possible.

The problem is that we're probably going to need to include the app name in the URLs and then check on that. That will not only break packages out there but also people's custom code and will limit moving trees around in trees.config so it's got a big impact.

If you need a fix asap then you're more then welcome to create a custom build, but you're likely going to run into similar problems. Although if someone can come up with a backwards-compatible solution then that would be awesome of course!

FYI (and I know, that's security by obscurity) this has never before been exploited that we know of, so the risk is pretty low.


Shannon Deminick 03 Apr 2013, 17:00:06

@Anthony, here's the solutions that have been proposed and the impact surrounding them...

Solution #1 - we need to pass in an 'app' query string to the create.aspx page and also add an 'app' parameter to the ajax calls like legacyAjaxCalls.delete. Then we can authorize the current user based on the contextual 'app' that they are currently working in. It is easy enough to pass in this 'app' parameter internally but some packages are launching the create.aspx page and calling these ajax methods directly and enforcing the app parameter will break these packages. The interim solution to this is to create a list of packages that are doing this an assigning a 'default app' internally to them until they update their codebase. BUT, this does not actually fix the security issue because the malicious user can just pass in the alias of an app that they know they are authenticated for in the 'app' parameter. So to get around this issue we can encrypt the 'app' parameter which is easy enough, however this still does not make it secure because the malicious user would be able to identify through trial and error what encrypted value matches an app alias that they are authenticated for by analyzing the JSON returned from the tree service.

Solution #2 - we do the security check in the ITask implementation which controls how things are created/deleted for a particular section/tree. This then presents us with other issues because we would not be able to fix this security issue for any custom packages, we'd have to get all package developers to release a new version of their package that adheres to the new security rules. We can give them a nice base class to inherit from but they'd still need to modify their own codebase for this to work. The other problem with this is that we will be essentially locking down a section to an app, meaning a developer would not be able to modify the trees.config file and move/copy a tree to a different app.

Solution 1 is more ideal since this breaks a lot less things and still allows for sections to be moved among apps but it poses a very tricky solution to figure out how to authorize a user based on a contextual app that is 100% secure and not just an obfuscated security check and also keeping in mind that one user can be working consecutively on several different apps at once by having multiple tabs open. Solution #2 is 100% secure but again this means that we will be locking down all ITask's to a particular app.

If there's a will there's a way so I might not have though of something obvious, any input and ideas would be great!


Shannon Deminick 03 Apr 2013, 18:50:23

What I've just come to realize in our codebase however is that having the ability to move trees/sections to different apps already will not work!! For example, we have a property called CurrentApp which is part of the UmbracoEnsuredPage. This is set in all overridden instances of UmbracoEnsuredPage where necessary (it is a poor implementation but it is there). This means that anywhere that CurrentApp is specified, the editor is 100% locked down to that application. If someone attempts to move the tree to another application the page will authenticate the user based on the hard coded application, not the application that it is trying to execute under.

As an example, if you have a user named 'Tom' who only has access to a custom app called 'Stuff' and you wanted to put the Media tree in to that application... well it's not gonna work because the editMedia.aspx page in it's ctor already hard codes that the current app is Media: CurrentApp = BusinessLogic.DefaultApps.media.ToString(); And the base class UmbracoEnsuredPage will enforce this security.

So i guess just locking things down by ITask will be ok since moving tree's to different apps won't behave as expected anyways based on the security model of assigning users to apps. What is odd though is that in our codebase we are not assigning the CurrentApp to the editContent page... I'm wondering if that is because someone somewhere is loading the editContent page in another app because they don't want to give a user access to the 'Content' app?

So now what I've come to realize is that our security model is actually flawed, we should really be assigning security based on both the app and the actual section/tree. Security by app will literally just disable the user from viewing the entire app but security by section will actually secure these resources. Then moving a section/tree to another app will work just fine and the security model will also work 100%. However, the implementation of this would be complex and we'd have to ask all package developers to update their codebase.


Anthony jones 03 Apr 2013, 19:35:34

Shannon. Thanks I really appreciate the analysis. So where does that leave me? Are my options to hire Umbraco to make a fix for me to the 4.11.x release with custom code and not share it with the community at this point or I could take it on myself?


Shannon Deminick 03 Apr 2013, 19:52:54

@Anthony, to immediately fix this issue you would need to put the authorization logic into each of the ITask implementations which will lock the section/tree to that application. If for some reason a package is sharing an ITask implementation for a section in another app you'll have problems but I highly doubt that is happening. You'll also have issues with custom package sections that you install because their ITask implementations won't be secured.

I've listed the above analysis to be transparent about our options but as you will notice we haven't come up with the correct solution yet otherwise we'd already be fixing this.

To fix this properly if we go down the route of patching all ITask implementations for a release, we'd have to:

  • Create a new interface & base class - something like: IAuthorizeTask, BaseTask
  • Make a list of all popular packages that have their own sections and create an internal list to maintain what nodeType aliases that are found in the UI.xml are supposed to be in which 'app' creating a 1:1 relation.
  • Then any ITask that does not implement IAuthorizeTask will be proxied to our custom LegacyAuthorizeTask to validate against our internal list
  • This internal list can have an API for developers to add custom aliases/app since we won't have every package's node aliases already integrated.

We've been discussing this internally to come up with the best solution moving forward, it's just not simple task based on how many people this could affect.


Anthony jones 03 Apr 2013, 19:56:01

Ok got it. So its a beast and no immediate action can be taken.


Shannon Deminick 03 Apr 2013, 20:00:00

It just requires a little more thought is all, but you could patch your install based on the solution I just mentioned.


Anthony jones 03 Apr 2013, 20:12:57

Just one more question. Could I hire you guys to fix this issue and the linked issue reflective XSS for us?


Anthony jones 04 Apr 2013, 14:37:26

If we take this on ourselves I need a suggested resolution for the reflective XSS issue as well? Shannon not trying to be a pest but preferably is there a way to outsource these to the core Umbraco team or other such as a support mechanism or someone I can contact that I can contract this through for the vertical escalation bug and reflective XSS issue?


Sebastiaan Janssen 04 Apr 2013, 14:41:49

@Anthony Yes, this is what the Confidence program was designed for: http://umbraco.com/products/confidence.aspx


Anthony jones 04 Apr 2013, 18:19:31

Ok Sebastiaan thanks. So this may seem a bit odd. My client has a very tight deadline as this pen test was consucted right before launch. They are a large corporation and have gone through the procurement process to purchase the support but wont actually send the money until Monday. i have docuemnted proof of that. Is there any way this can be worked on immediately if I supply the proof?

These are 3 critical issues Vertical escalation Reflected XSS Installing Umbraco in a subdirectory (e.g. XXX.ZZZ.com/catalog)

Would this be acceptable?


Sebastiaan Janssen 04 Apr 2013, 19:12:37

I'm sorry but we have to be pretty strict with our terms and services (been bitten before): http://umbraco.com/media/168711/umbraco_support_terms_and_conditions.pdf

Please note that we allow one open issue at a time to be sent through the umbraco.com account that "owns" the confidence license (see document above as well).

Make sure to detail the problem you have with running in a subdirectory as well, Umbraco should run fine but maybe you've found something new.


Anthony jones 05 Apr 2013, 14:38:23

Ok Sebastiaan I have the support contract active and have re-submitted the issues via support ticket. Also Shannon's suggestion for repairing he stated that custom package may not be secure - the only custom package we are using is image resizing. So whats next?


Sebastiaan Janssen 05 Apr 2013, 15:36:49

@Anthony Perfect, thanks! I'll follow up in Zendesk. As I've already mentioned we'll be going through these one at a time, so I'll assign priorities as follows:

  1. XSS attacks
  2. Privilege escalation
  3. Virtual directory support

We'll keep you posted on the progress, number 1 is being worked on as we speak.


Sebastiaan Janssen 05 Apr 2013, 15:52:39

Please also note that feedback and questions will be in Zendesk as of now, so please monitor the email address of the account associated with the Confidence license, thanks!


Anthony jones 05 Apr 2013, 16:16:55

I dont have access to that email. Any way I can get that correspondence also?


Anthony jones 05 Apr 2013, 17:05:14

Just want to be sure we are working on version 4.11 correct?


Shannon Deminick 05 Apr 2013, 17:07:11

@Anthony, I've added you to the cc list of the ticket and yes currently the codebase being modified is the 4.11.7 branch.


Sebastiaan Janssen 05 Apr 2013, 17:10:12

@Anthony please check with the people reading that email account for replies I've already sent, have some questions about the virtual directory issue.


Anthony jones 05 Apr 2013, 17:15:26

I can try anyway you could post those questions here?


Sebastiaan Janssen 05 Apr 2013, 17:38:16

@Anthony re-sent through zendesk with a CC to you


Anthony jones 05 Apr 2013, 17:39:53

Thanks have it and getting back to you.


Shannon Deminick 08 Apr 2013, 19:08:03

ITasks are secured in bcfee3b9c962 (vertical escalation issue)


Priority: Major

Type: Bug

State: Fixed

Assignee: Shannon Deminick

Difficulty: Normal

Category: Security

Backwards Compatible: True

Fix Submitted:

Affected versions: 4.8.0, 4.9.0, 4.10.0, 4.11.0, 6.0.0, 4.9.1, 4.11.1, 4.11.2, 4.11.3, 4.11.4, 6.0.1, 4.11.5, 6.0.2, 4.11.6, 6.0.3

Due in version: 6.0.4, 4.11.7

Sprint:

Story Points:

Cycle: