search

ClassicPress / classicpress-seo

Classic SEO is the first SEO plugin built specifically to work with ClassicPress. A fork of Rank Math, the plugin contains many essential SEO tools to help optimize your website.
GNU General Public License v2.0
30 stars 10 forks source link

Database error when activating CPSEO #87

Closed nylen closed 4 years ago

nylen commented 4 years ago

This error occurred upon activating the plugin, and it looks like it will re-appear up to once per day:

2020-07-24T15-06-28Z

The problem is on this line:

https://github.com/ClassicPress-plugins/classicpress-seo/blob/2a02cf4ffd2576988671f89553d717282c44289b/includes/modules/search-console/class-db.php#L338

The wrong kind of quotes are used (double quotes instead of single quotes for a value). This code should also be using $wpdb->prepare instead of concatenating values manually.

I did not look more closely to see what this code is trying to do or whether it is actually used anywhere.

timbocode commented 4 years ago

Just to confirm, this code generates the Cached Days, Data Rows and Size figures on the Search Console settings page (General Settings -> Search Console).

image

Props to @nylen for the following.

The error is caused by the use of double quotes instead of single quotes, which is invalid SQL syntax. For example:

WHERE table_schema="www"

In many MySQL installations, no error would be produced but the error is generated when the ANSI_QUOTES SQL_MODE flag is set. With ANSI_QUOTES enabled, you cannot use double quotation marks to quote literal strings because they are interpreted as identifiers. See https://dev.mysql.com/doc/refman/8.0/en/sql-mode.html#sqlmode_ansi_quotes.

Many hosting companies do not enable ANSI_QUOTES by default. However, DigitalOcean is one hosting company that does. As a result, this error will be seen on DO installations.

Changing the double quotes to single quotes will resolve this issue.

In addition, the code should be updated to use $wpdb->prepare which prepares the SQL query for safe execution and prevents against SQL injection attacks.

I've prepared this gist https://gist.github.com/timbocode/b75a47022fb0792dfb9d93d76b9b294e which I hope addresses the above.

nylen commented 4 years ago

I don't see anything for "Search Console" under "General Settings". I am also not able to trigger this error again. Need to stop for now, I'll take another look at this later...

nylen commented 4 years ago

Ok, found it, Search Console needs to be activated/enabled under Dashboard first.

I did this a little differently than your gist. Upon a closer look there wasn't really any risk of SQL injection here since no user input goes into these queries. However, as a best practice any time you have WHERE x = '$variable' in a query then the $variable should be a placeholder handled by a prepared statement. The same isn't really true for table names, since these are (hopefully!) never going to be decided based on user input, and if we were using "real" prepared statements then putting a placeholder there wouldn't work anyway.

PR up at #97.