'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]

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)

[Cadyshack]

I approve this pull request. It fixes an important issue. Tested this solution in my own project and fixed the problem caused by SELECT FOR UPDATE queries.

Apparently my approval means nothing, but if someone with proper permission could approve this, that would be great. HyperDB 1.9 is unusable without this change in my case, and many others.

[sonarqubecloud]

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.

[sonarqubecloud]

Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud