Make WordPress Core

Opened 3 years ago

Last modified 5 months ago

#53965 new defect (bug)

Legacy widgets used inside a normal block fail when salts are changed

Reported by: enchiridion's profile Enchiridion Owned by:
Milestone: Awaiting Review Priority: normal
Severity: major Version: 5.8
Component: Widgets Keywords:
Focuses: administration, rest-api Cc:

Description

When a legacy widget is added to the new block based widget area and it's a 1st level block, its data is stored the way widgets have always stored data. However if a legacy widget block is used inside of a normal block its data is no longer stored separately in the options table, but is encoded and stored with its parent block.

The legacy widget data is stored by base64 encoding its serialized data then generating a hash of it using wp_hash(). Both pieces of data are stored in the parent block. When viewing the widget block editor this data is hydrated. The problem is the default scheme AUTH is used for wp_hash() when none is specified. The value of AUTH is volatile. Salt keys should be rotated occasionally and they can be changed via the salt filter, when that happens all the embedded legacy widgets will break.

I can think of several ways to fix this:

  • Use a more stable salt.
  • Encode data using a method that doesn't rely on a hash for verification.
  • Remove the hash check. I don't see anywhere in the codebase (but I'm far from an expert) where serialized data is compared against a hash before sending it to unserialize(), so I don't think it adds any more risk as it is supposed to be trusted data. To me this is the cleanest solution, the first two methods would require a backwards compatibility fix: if the data failed to unserialize the new way, try the old method.

The hash check to remove is in \wp-includes\rest-api\endpoints\class-wp-rest-widget-types-controller.php on lines 447 - 453. A couple other places in the method also need the hash references removed. Luckily the encode_form_data() method is new and only used here so any changes won't be painful.

I originally came across this issue because my new WP 5.8 site was having widget errors whenever it was migrated to the staging server. My local server and staging server have different salt values, so it caused all the nested embedded widgets to fail. I was using some Navigation Menu widgets, which failed with the error The "nav_menu" block was affected by errors and may not function properly. Check the developer tools for more details. In dev tools this was the REST response {"code":"rest_invalid_widget","message":"The provided instance is malformed.","data":{"status":400}}.

I verified the differing AUTH_KEYs were the issue by syncing them, which temporarily enabled the legacy widgets to work, however that isn't a realistic fix.

Change History (5)

#1 @Enchiridion
3 years ago

My workaround was to create a custom Menu block so I didn't need to use the legacy one. Oh, that is another way to fix the issue. Create a modern block then upgrade any legacy menu widgets to it whenever they are edited again.

Last edited 3 years ago by Enchiridion (previous) (diff)

#2 @kraftner
3 years ago

This is even worse for deployments that, for security reasons, involve an automatic Salt rotation on each deploy. It practically makes the whole Widget Editor unusable. :(

#3 @roundhousedesigns
3 years ago

  • Severity changed from normal to major

+1 here. We run 100+ WP installations in Docker stacks, and whenever the containers restart the salts shuffle, breaking some nav_menu (and sometimes other legacy) blocks -- which is used on a lot of our sites! I'd rather not have to prevent salts from shuffling, but that seems to be my only recourse right now.

I'm also noticing it tends to happen on nav_menu widget blocks that are inside Group or Column blocks.

Last edited 3 years ago by roundhousedesigns (previous) (diff)

#4 @CockpitSeeker
3 years ago

+1 This is big and was unavoidable here after switching to the block system with pre-existing legacy widgets. I would go as far as saying this is a major deterrent against the whole block paradigm adoption. Suffice to say, at this date the "Classic Widgets" plugin has 900,000+ installs...

#5 @gmariani405
5 months ago

Still an issue in 2024

Note: See TracTickets for help on using tickets.