WSOD protection should disable plugins in fewer situations

[markjaquith]

[44524] implemented WSOD protection, for #44458. I have concerns about how aggressively it pauses plugins.

It is possible to get a plugin paused in the admin by triggering a fatal error on the front of the site. Furthermore, that fatal error could be triggered by specific user input (maliciously, even). If this is a plugin that implements 2FA or SAML or admin event logging or permissions filtering or login attempt limiting or any other security- or authentication-related function, this could have dire effects on the site. Bad actors could have a way of targeting and disabling critical site functionality.

I think the scenarios where this pausing occurs should be narrowed. To start, I propose that pausing not happen for:

1. POST requests (on the front end).
2. Requests with a query string other than ?p=X (or a few other standard WordPress query strings we can whitelist).

For example, if a contact form submission, in certain circumstances, can result in a fatal error, this should not pause that plugin. If some ?obscure-query-arg=BAD_INPUT URL on a site can result in a fatal error, this should not pause the plugin.

If a plugin is taking down the entire site, or preventing a user from logging in, that is the dead end scenario we'd like to prevent, and those are the situations in which it's an acceptable trade-off to pause that functionality so that the site owner can take steps to recover more gracefully than they could in the past.

See also #45888 which proposes that critical plugins (where the plugin itself knows that it's better to take the site down than have its functionality disabled) have a way of opting out of WSOD protection.

[TimothyBlynJacobs]

I think we can also limit pausing extensions to only errors that occurred on pages where is_protected_endpoint() is true. We could continue to log these errors but display them differently in the admin.

[flixos90]

45940.diff​ separates the processes of detecting an error to handle and actually storing it (and by that pausing the extension). This provides us with the baseline to pause extensions more granularly. I also included the suggestion by @TimothyBlynJacobs already since that enhancement makes sense and is rather trivial to implement.

I will commit this now to have it in the next Beta, and we can continue working from there, also considering "irregular" requests for not pausing extensions, as suggested by @markjaquith.

[flixos90]

In 44623:

Bootstrap/Load: Only pause extensions when they cause a crash on a protected endpoint.

This is a first step on pausing extensions less aggressively. If a plugin or theme only causes a crash in the frontend, there is no point in pausing it in the admin backend.

See #45940, #44458.

[flixos90]

Flagging this for a follow-up patch to implement the initial concerns outlined in the ticket description.

[flixos90]

Note that by not always pausing extensions when an error occurs, there currently is no longer any awareness of the error either. I wonder whether we should decouple pausing extensions from still storing the error in other cases so that the site owner can be informed by it.

Due to the time constraints, this specific part could be an enhancement for 5.2 (or possibly even a 5.1.x) though, as the other fixes are more critical.

[schlessera]

I wonder whether we should decouple pausing extensions from still storing the error in other cases so that the site owner can be informed by it.

Isn't this already handled by WP_DEBUG_LOG? I don't think we should build a complete developer/debugger tool here, just make sure people can run updates/removals on their plugins/themes.

[schlessera]

Added a mechanism to only paused plugins/themes after an initial redirect, to make sure the error is persistent: https://core.trac.wordpress.org/ticket/46066

[flixos90]

In 44717:

Bootstrap/Load: Revert fatal error recovery mechanism from 5.1 to polish for 5.2.

Due to the high number of follow-up tickets and associated security concerns, it was decided to reschedule the fatal error recovery feature for WordPress 5.2, in order to address these issues properly. The feature will continue to be developed, with iterations being merged into trunk early in the 5.2 release cycle.

Fixes #46141. See #44458, #45932, #45940, #46038, #46047, #46068.

[flixos90]

This ticket is based on the old fatal error recovery mode implementation and will be covered as part of #46130.