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

Security Scan reports vulnerable TinyMCE version lib. #10217

Closed
MrenGit opened this issue May 3, 2021 · 9 comments · Fixed by #10653
Closed

Security Scan reports vulnerable TinyMCE version lib. #10217

MrenGit opened this issue May 3, 2021 · 9 comments · Fixed by #10653
Milestone

Comments

@MrenGit
Copy link

MrenGit commented May 3, 2021

A automated security scan reports TinyMCE 4.9.11 as a vulnerable version, and recommends upgrade to 5.4.0
The lib seems to be detected on the Umbraco Login page.

Are there any plans of upgrading the library?

Umbraco version

Umbraco 8.11.1, but also seems to be the case in 8.13.0

@emma-hq
Copy link
Contributor

emma-hq commented May 4, 2021

Hi @MrenGit

Thanks for bringing this to our attention. We'll take a look and let you know what we decide.

Emma

@emma-hq emma-hq added the state/needs-investigation This requires input from HQ or community to proceed label May 4, 2021
@MMasey
Copy link
Contributor

MMasey commented Jun 1, 2021

Heyo,

Is there any update on this, it's currently a pretty major blocker for one of our projects.

@nul800sebastiaan
Copy link
Member

@MrenGit We previously looked into an upgrade and it is currently infeasible to upgrade to v5 of Tiny: https://github.com/umbraco/rfcs/blob/main/cms/0016-TinyMCE-RTE-Improvements.md

@MMasey

There are 2 vulnerabilities here (with the same root cause), that are both marked with "moderate severity".

  1. GHSA-w7jx-j77m-wp65
  2. GHSA-5vm8-hhgr-jcjp

Both are due to a lack of URL sanitation. Number 2 doesn't affect standard installs of Umbraco, we don't allow form as an element by default. We do, however allow object, iframe and embed as described in the first report.

If this is a blocker for you, the fix here is to remove object, iframe and embed as validElements in tinyMceConfig.config and then recycle the application pool. I can't foresee any immediate effects except that the Embed button in the editor will no longer work, this uses iframes to do the embedding and that is no longer allowed.

What is needed for an attack?

The one thing we can imagine is that a user in the backoffice with intimate knowledge of both this TinyMce vulnerability AND intimate knowledge of the inner workings of Umbraco could potentially be able to devise a URL which, when executed by an admin, might elevate the editors user account to admin. It has to be noted that this is currently a theoretical attack, we don't know if this is actually possible.
Do note that this requires a logged in user, meaning someone you already put some level of trust in, and whom you've willingly invited to use the backoffice of Umbraco (in other words: there is a clear audit trail of who exactly would be trying to exploit vulnerabilities).

Our rating

For now, we rate the severity as "low" since

  • This requires an authenticated user in the backoffice of Umbraco
  • This requires quite a lot of specialized knowledge

However, over time someone might create a simple URL that leads to an exploit, so this should be addressed in how we configure TinyMCE.

As such, we think it's safe to use the current version as is but we'll have to discuss with the team what mitigations we could put in place, using the guidance provided by Tiny in both vulnerability reports.

@MMasey
Copy link
Contributor

MMasey commented Jun 1, 2021

Thanks so much for this @nul800sebastiaan, that is exacly what I needed 😃

@MMasey
Copy link
Contributor

MMasey commented Jun 30, 2021

Hey @nul800sebastiaan, hope all is well. This is more of a formality than anything due a client wanting to follow up on this issue, but are you able to provide a potential timing plan for a fix for this issue, or even if there is one.

Thanks

P.S. I know the open source nature of Umbraco makes this a potentially impossible question, as I said it's more of a formality than anything.

@nul800sebastiaan nul800sebastiaan added state/in-sprint We've committed to work on this during the sprint indicated in the milestone and removed state/needs-investigation This requires input from HQ or community to proceed labels Jul 12, 2021
@nul800sebastiaan nul800sebastiaan added this to the sprint166 milestone Jul 12, 2021
@nul800sebastiaan
Copy link
Member

Sorry I didn't see your query @MMasey - this is in our current sprint now to look into.

@orsinic
Copy link

orsinic commented Aug 10, 2021

Hi @nul800sebastiaan, any news about the investigation at your side by chance? In one of our projects this vulnerability was also raised during a security scan.

The workaround you mentioned is highly appreciated, but this unfortunately does not avoid the vulnerability being raised in new scans.

Thanks in advance!

@nul800sebastiaan nul800sebastiaan removed the state/in-sprint We've committed to work on this during the sprint indicated in the milestone label Nov 3, 2021
@nul800sebastiaan
Copy link
Member

Fixed in #10653 - make sure to read the details there.

For existing sites this will require active opt-in as it changes sanitization behavior which might affect your workflow. For new installs the default will be that you are opted in to the additional sanitization.

@nul800sebastiaan
Copy link
Member

Please do note: we will still get the same results in the security scans, since we can not upgrade to Tiny version 5 at this time. So when relevant you can point to this issue and the related pull request.

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 a pull request may close this issue.

5 participants