← Back to context

Comment by dinkelberg

1 day ago

What an awful fix. Does WordPress seriously still use basic string concatenation (edit: and sprintf) to build SQL queries?

14 comments

dinkelberg

Reply
Yokohiii  1 day ago

To construct dynamic sql queries to have to string concatenate at least some parts, .

User data should of course be passed via prepared statements.

  • codedokode  1 day ago

    One usually uses "query builder" pattern for that.

    Also, regarding placeholders, historically many DB and frameworks do not support passing lists for a value in a placeholder (like "WHERE id IN(?)") so users of such software fall back to string concatenation.

  • formerly_proven  1 day ago

    Not user code, no. Someone eventually has to, but virtually every ORM under the sun allows you to construct dynamic queries without having to concatenate strings yourself or resort to string interpolation.

    • winstonwinston  1 day ago

      Quick search shows this in Wordpress:

      > WordPress database access abstraction class.

        class wpdb {}

      So this is some sort of ORM provided.

        $results = $wpdb->get_results( "SELECT * FROM {$wpdb->prefix}options WHERE option_id = 1", OBJECT );

      > Some of the methods in this class take an SQL statement as input. All untrusted values in an SQL statement must be escaped to prevent SQL injection attacks. Some methods will escape SQL for you; others will not. Check the documentation to be sure before you use any method in this class. For more on SQL escaping in WordPress, see the section entitled Protect Queries Against SQL Injection Attacks below.

      It does not however prevent $wpdb users from NOT binding query parameters, which leads to this vulnerability.

      1 reply →

reddalo  1 day ago

WordPress source code is a mess. They should re-write it from scratch using modern technologies, or even a framework like Laravel.