Make WordPress Core

Opened 4 years ago

Last modified 2 years ago

#51486 new defect (bug)

The add_option function should not be able to update existing rows in the database.

Reported by: khag7's profile khag7 Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version: 2.9
Component: Database Keywords: needs-patch needs-unit-tests
Focuses: Cc:

Description

In certain edge cases, add_option is able to update existing option values. This should not be possible. If SQL is executed which instructs "Insert XYZ" into database, then the insert should fail if the key already exists in the database, unless the SQL specifies that an UPDATE should happen.

The add_option function does check first to see if an option exists before attempting to add it to the database. In the overwhelming majority of cases, this works fine because when the option exists the function will return early before attempting to insert into the database.

However, consider a scenario where an object cache is being used. And further consider that the object cache may report incorrectly. If the object cache tells add_option that the value doesn't exist in the database (but in reality it does!), then add_option continues on and attempts to insert the supposedly new option.

In that particular circumstance, the SQL query, shown below, is executed which will insert the new row. And in the event that the option already exists, it will be overwritten.

The issue here is that add_option shouldn't be able to update the existing value. The query should fail. Why is the ON DUPLICATE KEY UPDATE clause included here?

<?php    // Code here is from wp-includes/option.php Line 581:
    $result = $wpdb->query(
        $wpdb->prepare(
            "INSERT INTO `$wpdb->options` (`option_name`, `option_value`, `autoload`)
             VALUES (%s, %s, %s)
             ON DUPLICATE KEY UPDATE
                `option_name`  = VALUES(`option_name`),
                `option_value` = VALUES(`option_value`),
                `autoload`     = VALUES(`autoload`)
            ",
            $option,
            $serialized_value,
            $autoload
        )
    );

I realize this is an edge case. If the object cache being used gives bad information to add_option then really its the object caching plugin at fault. However, I can't understand a possible circumstance where add_option should ever UPDATE an existing option. The SQL here should not include ON DUPLICATE KEY UPDATE. Am I missing something? Is there a good reason for that clause? What would happen if that clause were removed?

Change History (3)

#1 @khag7
4 years ago

And now for a little history...

It was 11 years ago, and add_options was located in wp-includes/functions.php back then.

The clause ON DUPLICATE KEY UPDATE was added to the add_option function in ticket #11437 and commit [12403]

The description in that ticket does explain why this clause exists in the SQL. On a busy site, you can't know whether the option exists or not before attempting to insert it. As such, an INSERT query may fail.

Unfortunately, they solved that ticket incorrectly. The solution should have been that whatever piece of code was attempting to add the option site_transient_timeout_theme_roots should have been using update_option instead. This would've given them the result they wanted. Instead, they patched add_option with ON DUPLICATE KEY UPDATE and now we have a function that is intended to add and never update, but which in fact does update.

#2 @markparnell
4 years ago

  • Component changed from General to Database
  • Keywords needs-patch needs-unit-tests added
  • Version set to 2.9
Note: See TracTickets for help on using tickets.