4 ms·

> Where is the CVE? What risk is there continuing to use the original plugin? Here’s the diff showing what has changed: https://plugins.trac.wordpress.org/chan

by maxloh 2y ago

> Where is the CVE? What risk is there continuing to use the original plugin? Here’s the diff showing what has changed: https://plugins.trac.wordpress.org/changeset?new=3167679%40advanced-custom-fields&old=3143016%40advanced-custom-fields#diff-legend https://plugins.trac.wordpress.org/changeset?new=3167679%40a...

grayhatter 2y ago
wow, they deleted 300 lines, many giving credit to others, just to replace it with > Security - ACF defined Post Type and Taxonomy metabox callbacks no longer have access to $_POST data. (Thanks to the Automattic Security Team for the disclosure) If I was on that security team, I would be livid they used my team's name on this behavior. If this was done by that security team, their ethics are disgusting, and likely non-salvageable... Still looking for the security exploit worthy of a plugin takeover though. edit; best I can figure tonight is it's some concern over CSRF, but they don't even sanitize $_GET nor $_SESSION, only _POST and _REQUEST... so either it's more complicated than it looks on the surface, or this "fix" is partial at best, and wasn't written by someone from security. (It's also possible or likely that I'm missing some context, it's been a long time since I've had to work on php)
- luckylion 2y ago
  It's extra funny when you consider all the recent acts where some plugin developer sells his plugin to some shady company, they go and add "functionality" to it that fundamentally changes the plugin, adds forced widgets to users' websites to promote their services etc, and the WP security team is like "they're allowed to do that, that's perfectly fine". I don't know how much overlap there is between the Automattic and WP security teams but I assume there's some like with most things in WP.
- maxloh 2y ago
  Wieldy enough, that line likely came from the original developer (ACF/WP Engine) [0]. I believe WordPress.org backported the change and named it v6.3.6.1 at that time [1], before rebranding ACF in a later version (v6.3.6.2). [0]: https://www.advancedcustomfields.com/changelog/ https://www.advancedcustomfields.com/changelog/ [1]: https://plugins.trac.wordpress.org/changeset?new=3164480%40advanced-custom-fields&old=3143016%40advanced-custom-fields#diff-legend https://plugins.trac.wordpress.org/changeset?new=3164480%40a...
  - Svip 2y ago
    Ah, that would explain why those fixes are noted as "since 6.3.8".
- deleted 2y ago
  [deleted]
- cutemonster 2y ago
  Yes, the whole changelog: gone. That's weird. And: `Author: WP Engine` is now `Author: WordPress.org` Is that even legal? If they had changed to "Maintained by" it would make more sense.
- maxloh 2y ago
  Confirmed: that patch was actually authored by ACF. https://x.com/WordPress/status/1845698272888811775 https://x.com/WordPress/status/1845698272888811775
Svip 2y ago
- public $version = '6.3.6'; + public $version = '6.3.6.2'; Why would you break the version standard? Maybe there are scripts expecting a specific format or some such. Also, given the majority of this change, I think 6.4.0 or even 7.0.0 is more in order, but at least 6.3.7. Edit: To make matters worse, elsewhere in the code, it's referenced as 6.3.8. Very confusing.
Genbox 2y ago
For those reviewing the changeset: There are two places where they read a value directly from $POST into an $args array. There is no validation applied, which means an attacker can inject whatever value they wish.
- sschueller 2y ago
  In 2024, wtf. How can anyone especially on software with this kind of reach still do such absolute amateur things?
  - echoangle 2y ago
    I can't speak for WP Engine devs specifically but for Wordpress customization developers, the ones I've worked with were just absolute amateurs. The people doing this are mostly untrained people hacking together stuff layer upon layer until it kind of worked. Don't even ask about using version control. I don't want to say this applies to every Webdev but it attracts a group of people that aren't too much into IT but want to quickly learn programming to have a job.
  - criley2 2y ago
    > want free software to run website > want free plugins to add functionality to site > absolutely will not hire developers or pay for software or plugins > how dare the free plugin for my free software not be coded to the highest standards
    - luckylion 2y ago
      That code is identical to the commercial version ACF Pro which currently costs $49/yr for a single website.
      - sieabahlpark 2y ago
        [dead]
- talkin 2y ago
jeroenhd 2y ago
I do see various security fixes in that patch, but most of the changes are removing references and code for a "pro" version of the plugin. I'm guessing the WP security team has been pentesting any WPEngine code they could get their hands on to find an excuse to make all of these changes. The security issues do look bad (once again proving that WordPress' worst vulnerabilities come from the plugins they install) but I think the branding removal is pretty wild. A quick skim through the plugin development guidelines does seem to indicate that trialware isn't allowed, and the plugin seems to be doing all kinds of other stuff that isn't really permitted by the guidelines. I don't know if WordPress is as strict in enforcing those as they are with this plugin, but the changes do seem to be based on them. With WPEngine recommending people to install their (vulnerable) version, I once again feel like there's no right side in this conflict. What a mess.
- maxloh 2y ago
  WP Engine fixed the vulnerability about a week ago [0]. The WP security team may have just backported the fix, even using the same line in their changelog [1]. [0]: https://www.advancedcustomfields.com/changelog/ https://www.advancedcustomfields.com/changelog/ [1]: https://plugins.trac.wordpress.org/changeset?new=3164480%40advanced-custom-fields&old=3143016%40advanced-custom-fields#file590 https://plugins.trac.wordpress.org/changeset?new=3164480%40a...

by maxloh 2y ago

grayhatter 2y ago

wow, they deleted 300 lines, many giving credit to others, just to replace it with > Security - ACF defined Post Type and Taxonomy metabox callbacks no longer have access to $_POST data. (Thanks to the Automattic Security Team for the disclosure) If I was on that security team, I would be livid they used my team's name on this behavior. If this was done by that security team, their ethics are disgusting, and likely non-salvageable... Still looking for the security exploit worthy of a plugin takeover though. edit; best I can figure tonight is it's some concern over CSRF, but they don't even sanitize $_GET nor $_SESSION, only _POST and _REQUEST... so either it's more complicated than it looks on the surface, or this "fix" is partial at best, and wasn't written by someone from security. (It's also possible or likely that I'm missing some context, it's been a long time since I've had to work on php)

luckylion 2y ago
It's extra funny when you consider all the recent acts where some plugin developer sells his plugin to some shady company, they go and add "functionality" to it that fundamentally changes the plugin, adds forced widgets to users' websites to promote their services etc, and the WP security team is like "they're allowed to do that, that's perfectly fine". I don't know how much overlap there is between the Automattic and WP security teams but I assume there's some like with most things in WP.
maxloh 2y ago
Wieldy enough, that line likely came from the original developer (ACF/WP Engine) [0]. I believe WordPress.org backported the change and named it v6.3.6.1 at that time [1], before rebranding ACF in a later version (v6.3.6.2). [0]: https://www.advancedcustomfields.com/changelog/ https://www.advancedcustomfields.com/changelog/ [1]: https://plugins.trac.wordpress.org/changeset?new=3164480%40advanced-custom-fields&old=3143016%40advanced-custom-fields#diff-legend https://plugins.trac.wordpress.org/changeset?new=3164480%40a...
- Svip 2y ago
  Ah, that would explain why those fixes are noted as "since 6.3.8".
deleted 2y ago
[deleted]
cutemonster 2y ago
Yes, the whole changelog: gone. That's weird. And: `Author: WP Engine` is now `Author: WordPress.org` Is that even legal? If they had changed to "Maintained by" it would make more sense.
maxloh 2y ago
Confirmed: that patch was actually authored by ACF. https://x.com/WordPress/status/1845698272888811775 https://x.com/WordPress/status/1845698272888811775

Svip 2y ago

- public $version = '6.3.6'; + public $version = '6.3.6.2'; Why would you break the version standard? Maybe there are scripts expecting a specific format or some such. Also, given the majority of this change, I think 6.4.0 or even 7.0.0 is more in order, but at least 6.3.7. Edit: To make matters worse, elsewhere in the code, it's referenced as 6.3.8. Very confusing.

Genbox 2y ago

For those reviewing the changeset: There are two places where they read a value directly from $POST into an $args array. There is no validation applied, which means an attacker can inject whatever value they wish.

sschueller 2y ago
In 2024, wtf. How can anyone especially on software with this kind of reach still do such absolute amateur things?
- echoangle 2y ago
  I can't speak for WP Engine devs specifically but for Wordpress customization developers, the ones I've worked with were just absolute amateurs. The people doing this are mostly untrained people hacking together stuff layer upon layer until it kind of worked. Don't even ask about using version control. I don't want to say this applies to every Webdev but it attracts a group of people that aren't too much into IT but want to quickly learn programming to have a job.
- criley2 2y ago
  > want free software to run website > want free plugins to add functionality to site > absolutely will not hire developers or pay for software or plugins > how dare the free plugin for my free software not be coded to the highest standards
  - luckylion 2y ago
    That code is identical to the commercial version ACF Pro which currently costs $49/yr for a single website.
    - sieabahlpark 2y ago
      [dead]
talkin 2y ago

jeroenhd 2y ago

I do see various security fixes in that patch, but most of the changes are removing references and code for a "pro" version of the plugin. I'm guessing the WP security team has been pentesting any WPEngine code they could get their hands on to find an excuse to make all of these changes. The security issues do look bad (once again proving that WordPress' worst vulnerabilities come from the plugins they install) but I think the branding removal is pretty wild. A quick skim through the plugin development guidelines does seem to indicate that trialware isn't allowed, and the plugin seems to be doing all kinds of other stuff that isn't really permitted by the guidelines. I don't know if WordPress is as strict in enforcing those as they are with this plugin, but the changes do seem to be based on them. With WPEngine recommending people to install their (vulnerable) version, I once again feel like there's no right side in this conflict. What a mess.

maxloh 2y ago
WP Engine fixed the vulnerability about a week ago [0]. The WP security team may have just backported the fix, even using the same line in their changelog [1]. [0]: https://www.advancedcustomfields.com/changelog/ https://www.advancedcustomfields.com/changelog/ [1]: https://plugins.trac.wordpress.org/changeset?new=3164480%40advanced-custom-fields&old=3143016%40advanced-custom-fields#file590 https://plugins.trac.wordpress.org/changeset?new=3164480%40a...