'FOR UPDATE' check in `is_write_query()`

[justinmaurerdotdev]

When using Wordfence's 2FA implementation together with HyperDB, the SELECT ... FOR UPDATE queries are currently getting interpreted as "not a write query", which causes user authentication to fail on read-only database replicas. My understanding is that SELECT ... FOR UPDATE is a write lock solution, so it requires a write-able instance.

Here's Wordfence's code, for reference, from wordfence/modules/login-security/classes/controller/totp.php:54:

$record = $wpdb->get_row($wpdb->prepare("SELECT * FROM `{$table}` WHERE `user_id` = %d FOR UPDATE", $user->ID), ARRAY_A);

I assume Wordfence isn't the only plugin to use write locks like this, so it seems like this "quick and dirty" is_write_query() is due for an upgrade. This fix appears to be working for me in production.

[sonarqubecloud[bot]]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
No Duplication information

[titustangible]

This causes coupon usage count queries to fail in Woocommerce as well

[dd32]

One downside of this PR is that SELECT * FROM table WHERE field LIKE '%example FOR UPDATE%' will be detected as a write query, which is an issue for WordPress search queries.

EDIT: After commenting, I realise HyperDB does support Transactions, but requires a specific comment to specify the data-set, if that comment is specified, although is_write_query() will return false, it should hit the write-server anyway: https://github.com/Automattic/HyperDB/blob/trunk/db.php#L325-L330 HyperDB doesn't really seem like it supports transactions (which SELECT FOR UPDATE is used within) as although BEGIN and START TRANSACTION are detected as write queries (since they don't look like a read-query) they'd go to the default HyperDB write server or fail as no table is specified as part of them. Once it's in a transaction, all future queries to that table should go to the write server.

Potentially what is needed here instead, is detecting a transaction starting and then treating all future queries until transaction end as a write query, even if it's a SELECT (such as in the case of what happens if you perform a write, and then start reading from the table)

[sonarqubecloud[bot]]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

[justinmaurerdotdev]

Circling back to this. It's still an issue in latest code, so I've updated the branch to make it merge-able again.