Page MenuHomePhabricator

CVE-2023-28447: Make a security release of Extension:Widgets due to Smarty RCE vulns
Closed, ResolvedPublicSecurity

Description

Someone just submitted a patch to bump smarty version - https://gerrit.wikimedia.org/r/c/mediawiki/extensions/Widgets/+/891299

Widgets is using a ~ operator on the version, so you may have an up to date version if you ran composer update recently.

However, the vulns in question are pretty bad. Not all of them apply to Widgets, but i was able to reproduce the RCE from CVE-2022-29221 (Assuming you can edit widgets). There were several other vulns listed in those versions, and i didn't look in detail. I think in practise many sites are vulnerable to this. At the very least, mediawikiwidgets.com appears to be (Based on Smarty version listed at Special:Version).

I think we should make a security announcement for this extension, and possibly a version bump to the extension in general (?).

Event Timeline

Bawolff changed Author Affiliation from N/A to Other (Please specify in description).Feb 23 2023, 2:30 PM
Bawolff added subscribers: Yaron_Koren, Kghbln.

It seems like the important release is 3.1.45. There is an XSS in 3.1.46, but its in the rather obscure mailto function, which is somewhat unlikely to be used, so while still definitely a security issue, is not so big a deal compared to the RCE that is in 3.1.44 (released in may).

So the issues in question are starting from 3.1.36 to present:

  • CVE-2018-25047 - XSS in mailto. Fixed in 3.1.47
  • CVE-2022-29221 - RCE in block name - Pretty bad. Would allow an attacker with widget editing rights to take over the server. Verified the PoC works on widgets. Fixed in 3.1.45
  • CVE-2021-21408 - RCE - Sandbox bypass for static methods - Pretty bad. Verified the PoC works on widgets. Would allow a widget editor to take over the server. Fixed in 3.1.43
  • CVE-2021-29454 - Math sandbox bypass. I did not verify this one on widgets, but sounds like an RCE. Possibly made worse by if a parameter was passed to the math plugin, maybe exploitable by people without widget editor, however i've never seen the math plugin used, so seems not likely in practise. Anyways pretty bad. Fixed in 3.1.42
  • CVE-2021-26119 - does not appear vulnerable in widgets configuration. fixed 3.1.38
  • CVE-2021-26119 - RCE function handling - verified PoC works with widgets. A widget editor can take over the server. fixed 3.1.38
Bawolff renamed this task from Make a security release of Extension:Widgets due to Smarty vulns to Make a security release of Extension:Widgets due to Smarty RCE vulns.Feb 23 2023, 3:17 PM

Ugh, 3.1.47 is marked as not supporting php8, so 3.1.46 is the highest you can get unless using php 7

So I +2'd the backports before realizing the php8 issue, which may have inadvertantly made this extension incompatible with php8 when using composer.

Is upgrading to the 4.x version of smarty an option here? I'm not sure what is different in it.

I made https://gerrit.wikimedia.org/r/c/mediawiki/extensions/Widgets/+/891576 to get master up to at least 3.1.46, which fixes the really scary vulns. The REL branches don't have php8 tests, so they merged the php8 incompatible update. Not sure if that should be reverted or what should be done here. Probably we should try to upgrade smarty to 4.x

@Bawolff - thanks for doing all this research. Did Smarty really drop compatibility with PHP 8 on a minor release? Or did Smarty 3.x never work with PHP 8, and they just neglected to note that until 3.1.47?

https://github.com/smarty-php/smarty/blob/support/3.1/CHANGELOG.md suggest the latter, but it is also unclear how unsupported "unsupported" really means.

Thanks for addressing this. I think we could definitely include this within the next supplemental security release (T325849) for a (likely) re-announcement. Since it's not Wikimedia-deployed or bundled with core, I'd guess a "security release" would just be noting the relevant gerrit change sets/backports and then making this bug public?

I was also thinking it would make sense to bump version and do a packigist release of widgets, so that people can know for sure that they have a secure version, but i defer to Yaron on how he wants to handle that.

I don't know anything about the Packagist stuff, but releasing a new version of Widgets definitely sounds like a good idea, if the Smarty update stuff is all done now.

I think maybe it makes sense to change it to allow smarty 4 at the same time, since smarty 3 allegedly doesn't work with php 8, and many people probably use php 8 at this point.

I don't know anything about the Packagist stuff

Whoops, my mistake, this extension doesn't have a packigist package, so no need to worry about that.

So the current situation is:

  • The 3.x version we need cannot be installed via composer on php8. Unclear how broken it really is on php8
  • We could use 3.1.46, which fixes the more serious vulnerabilities, but still has the XSS. So we really shouldn't do that.
  • We could mark it as needing 3.1.47, making the extension uninstallable via git (using composer) on php8, which seems bad
  • We could not use composer and instead bundle the dependency manually, in the hopes it really does work on php8.
  • We could update the extension to use smarty 4. This requires minor changes to WidgetSecurity. It also requires changing how Smarty resources are registered, as version 4 does not support registering resources via callbacks. Possibly it may require more changes/testing.

I'm not sure what we should do here, but we'd need to figure it out before announcing a new version.

  • We could use 3.1.46, which fixes the more serious vulnerabilities, but still has the XSS. So we really shouldn't do that.

I would concur that this is a bad option.

  • We could mark it as needing 3.1.47, making the extension uninstallable via git (using composer) on php8, which seems bad

This also isn't great, though it would at least prevent future installations or updates of vulnerable code.

  • We could not use composer and instead bundle the dependency manually, in the hopes it really does work on php8.

I think this is probably a bad idea.

  • We could update the extension to use smarty 4. This requires minor changes to WidgetSecurity. It also requires changing how Smarty resources are registered, as version 4 does not support registering resources via callbacks. Possibly it may require more changes/testing.

This sounds like an eminently reasonable solution depending upon your definition of the word "minor".

One thing to note here as well, is that commit 32993157584 which fixed a security issue is from Jan 2023, so most people who updated from that likely got the new version of Smarty at that time, since 3.1.47 was released in september 2022. However some people may have upgraded without running composer update (As evidenced by mediawikiwidgets.com) and may still be vulnerable.

So in the meantime until someone fixes the extension to be compatible with smarty 4, I'm going to make the REL branches look like master and do ~3.1.46 as a very short term solution and for consistency. (See also complaints at https://github.com/wikimedia/mediawiki-extensions-Widgets/commit/a13445f4a7e407f9cb440f85020a8bd6fe48fa16 ). There are zero widgets on mediawikiwidgets.com that use the mailto functionality, so as a very short term solution the risk seems ok.

Change 892026 had a related patch set uploaded (by Brian Wolff; author: Brian Wolff):

[mediawiki/extensions/Widgets@REL1_39] Very short term set smarty dependency to ~3.1.46

https://gerrit.wikimedia.org/r/892026

Change 891977 had a related patch set uploaded (by Brian Wolff; author: Brian Wolff):

[mediawiki/extensions/Widgets@REL1_35] Very short term set smarty dependency to ~3.1.46

https://gerrit.wikimedia.org/r/891977

Change 892026 merged by jenkins-bot:

[mediawiki/extensions/Widgets@REL1_39] Very short term set smarty dependency to ~3.1.46

https://gerrit.wikimedia.org/r/892026

Change 891977 merged by jenkins-bot:

[mediawiki/extensions/Widgets@REL1_35] Very short term set smarty dependency to ~3.1.46

https://gerrit.wikimedia.org/r/891977

I fully support upgrading this extension to use Smarty 4 - though I should note that I know nothing about Smarty registration, so I probably couldn't be the one to do it.

Change 892451 had a related patch set uploaded (by Brian Wolff; author: Brian Wolff):

[mediawiki/extensions/Widgets@master] Make forward compatible with Smarty v4.

https://gerrit.wikimedia.org/r/892451

Change 892451 merged by jenkins-bot:

[mediawiki/extensions/Widgets@master] Make forward compatible with Smarty v4.

https://gerrit.wikimedia.org/r/892451

Change 892970 had a related patch set uploaded (by Brian Wolff; author: Brian Wolff):

[mediawiki/extensions/Widgets@REL1_39] Make forward compatible with Smarty v4.

https://gerrit.wikimedia.org/r/892970

Change 892970 merged by jenkins-bot:

[mediawiki/extensions/Widgets@REL1_39] Make forward compatible with Smarty v4.

https://gerrit.wikimedia.org/r/892970

Mstyles renamed this task from Make a security release of Extension:Widgets due to Smarty RCE vulns to CVE-2023-28447: Make a security release of Extension:Widgets due to Smarty RCE vulns.Apr 3 2023, 10:47 PM
Mstyles claimed this task.
Mstyles moved this task from Watching to Our Part Is Done on the Security-Team board.
mmartorana changed the visibility from "Custom Policy" to "Public (No Login Required)".Apr 4 2023, 7:06 PM