Opened 7 years ago
Closed 5 years ago
#42477 closed defect (bug) (wontfix)
Cannot save theme customizer changes if nonce_life value is filtered in the active theme
Reported by: | figureone | Owned by: | |
---|---|---|---|
Milestone: | Priority: | normal | |
Severity: | normal | Version: | 4.8.3 |
Component: | Customize | Keywords: | |
Focuses: | administration | Cc: |
Description
Summary:
If the nonce_life
filter is used in the active theme to change the nonce lifetime from its default 86400 (e.g., to 3600), then changes made in the Theme Customizer cannot be saved.
Details:
WordPress 4.8 introduced some changes to the theme customizer, including this security-related commit that clears submitted post data if the nonce check fails:
https://github.com/WordPress/WordPress/commit/3d10fef22d788f29aed745b0f5ff6f6baea69af3#diff-681a4f686cf806e75ff8cd4c6d2fe5b7
This introduces an unintended side effect in a certain context:
- Since this code is hooked into
setup_theme
, it fires before the theme is loaded. - If a theme developer hooks into the
nonce_life
filter that can be used to adjust the lifespan of nonces, it will cause the nonce check above to fail (since the nonce ticks will not match).
https://developer.wordpress.org/reference/hooks/nonce_life/
https://github.com/WordPress/WordPress/blob/master/wp-includes/pluggable.php#L1999
This specific context happens in the theme customizer, when trying to Save & Publish changes. Interestingly, changesets can still be saved as an auto-draft on window blur (clicking outside of the browser window). I believe this is because changeset data is in $_POST['customize_changeset_data']
, while the same data is in $_POST['customized']
when the Save & Publish button is clicked.
I think the best solution here may be to remove the nonce_life
filter, but I don't know how widely it's used. Another possibility may be moving the nonce check out of WP_Customize_Manager::setup_theme()
, but I don't have a good idea at the moment for where it should go.
Change History (9)
#2
in reply to:
↑ 1
@
7 years ago
Replying to westonruter:
@figureone Interesting scenario. Why would a theme be adding a
nonce_life
filter in the first place?
Good question, and I don't have a great answer. :)
Once I diagnosed this issue, I simply removed the nonce_life
filter that a previous theme developer had written; their comments only pointed to:
https://wordpress.stackexchange.com/questions/94585/is-it-safe-to-assume-that-a-nonce-may-be-validated-more-than-once
So I gather someone might use it if they are worried about nonces being stolen and reused, but I don't think that is a great idea in a theme context.
The fix here could simply be to add some notes to the documentation for the filter; I just wanted to seek some input first to see if there were other uses for the nonce_life
filter that I wasn't thinking of.
#3
@
7 years ago
This was brought up in #41617 in a different way.
I think it's OK as-is, and the breakage is expected. If the theme filters nonce_life
globally and not per-action then it's deliberately changing how WordPress works, and it's similar to a theme replacing the Customiser with it's own version.
There's also #35188 to pass the nonce name through to aid in per-action filtering.
If you have a link to a theme (public, or paid) that triggers this, I'd be interested, feel free to post here or DM me on slack (@dd32)
I think the best solution here may be to remove the nonce_life filter
Personally I think the filter should be removed, or at least changed to only allow increasing the lifetime.
#4
follow-up:
↓ 6
@
7 years ago
Thanks @dd32, here's a simple demonstration theme (child theme of twentyseventeen) that simply changes the nonce_life value:
https://github.com/uhm-coe/twentyseventeen-child-demonstrate-nonce_life-bug
If you activate that theme and then go into the theme customizer, you'll see the behavior:
- Changing a value (e.g., Site Title), then clicking Save & Publish, then refreshing the page will show that the change was not made (it fails silently).
- Changing a value (e.g., Site Title), then blurring the page by clicking outside the window will successfully create a changeset. If you then click Save & Publish, the changeset does get published.
In tracing the code, I discovered the inconsistent behavior was because the ajax action for submitting changeset data uses $_POST['customize_changeset_data']
, while the ajax action for the Save & Publish button uses $_POST['customized']
. The core commit referenced in the OP only clears $_POST['customized']
if the nonce check fails.
#5
@
7 years ago
In regards to failing silently, this should be fixed in 4.9: a notification will appear when save errors occur.
#6
in reply to:
↑ 4
@
7 years ago
Replying to figureone:
Thanks @dd32, here's a simple demonstration theme (child theme of twentyseventeen) that simply changes the nonce_life value:
https://github.com/uhm-coe/twentyseventeen-child-demonstrate-nonce_life-bug
If you find any real-world example themes or plugins I'd be interested in those :)
@figureone Interesting scenario. Why would a theme be adding a
nonce_life
filter in the first place?