[wp-trac] [WordPress Trac] #60009: Potential SQL Injection in WordPress Core

[WordPress Trac]

#60009: Potential SQL Injection in WordPress Core
--------------------------+----------------------
 Reporter:  williamdee    |       Owner:  (none)
     Type:  defect (bug)  |      Status:  closed
 Priority:  normal        |   Milestone:
Component:  Query         |     Version:  3.1
 Severity:  normal        |  Resolution:  invalid
 Keywords:                |     Focuses:
--------------------------+----------------------
Changes (by swissspidy):

 * status:  new => closed
 * component:  Security => Query
 * version:  trunk => 3.1
 * milestone:  Awaiting Review =>
 * keywords:  needs-review =>
 * resolution:   => invalid
 * severity:  major => normal


Old description:

> **Action/s Required to Trigger (potentially):**
>
> When going to any WordPress URL (potentially).
>

> **File/s Involved (there are others):**
>
> wp-include/class-wp-query.php
>

> **Core Issue:**
>
> The SQL query to retrieve a page via post name uses generated SQL instead
> of static SQL with bound variables (lines 2031-2032):
>
> $q['name'] = sanitize_title_for_query( $q['name'] );
> $where    .= " AND {$wpdb->posts}.post_name = '" . $q['name'] . "'";
>
> **Note:**
>
> While the "sanitize_title_for_query()" function should sanitize this, if
> some hacker can slip something through that function then it is directly
> injected. For proper security the SQL should not be generated dynamically
> and should use bound variables.
>
> **Other Files Affected:**
>
> Using the following GREP brings up other lines to look at:
>
> grep -R " = ' \." *
>
> wp-includes/bookmark.php: $inclusions = ' AND ( link_id = ' . $inclink .
> ' ';
> wp-includes/bookmark.php: $inclusions .= ' OR link_id = ' . $inclink . '
> ';
> wp-includes/bookmark.php: $category_query = ' AND ( tt.term_id = ' .
> $incat . ' ';
> wp-includes/bookmark.php: $category_query .= ' OR tt.term_id = ' . $incat
> . ' ';
>
> Again, these should not be using dynamically generated SQL and should be
> using bound variables.

New description:

 **Action/s Required to Trigger (potentially):**

 When going to any WordPress URL (potentially).


 **File/s Involved (there are others):**

 wp-include/class-wp-query.php


 **Core Issue:**

 The SQL query to retrieve a page via post name uses generated SQL instead
 of static SQL with bound variables (lines 2031-2032):

 {{{
 $q['name'] = sanitize_title_for_query( $q['name'] );
 $where    .= " AND {$wpdb->posts}.post_name = '" . $q['name'] . "'";
 }}}

 **Note:**

 While the `sanitize_title_for_query()` function should sanitize this, if
 some hacker can slip something through that function then it is directly
 injected. For proper security the SQL should not be generated dynamically
 and should use bound variables.

 **Other Files Affected:**

 Using the following GREP brings up other lines to look at:

 {{{
 grep -R " = ' \." *

 wp-includes/bookmark.php: $inclusions = ' AND ( link_id = ' . $inclink . '
 ';
 wp-includes/bookmark.php: $inclusions .= ' OR link_id = ' . $inclink . '
 ';
 wp-includes/bookmark.php: $category_query = ' AND ( tt.term_id = ' .
 $incat . ' ';
 wp-includes/bookmark.php: $category_query .= ' OR tt.term_id = ' . $incat
 . ' ';
 }}}

 Again, these should not be using dynamically generated SQL and should be
 using bound variables.

--

Comment:

 Hi there and welcome to WordPress Trac!

 First of all, when creating this ticket you had to tick a checkbox saying
 that you are not reporting a security issue. If in doubt, please always
 responsibly report [http://make.wordpress.org/core/handbook/reporting-
 security-vulnerabilities/ security issues] to the
 [https://hackerone.com/wordpress WordPress HackerOne program].

 Second, please note that all of these occurrences, while assembling SQL
 queries, use wpdb for escaping queries before execution. It's the caller's
 job to sanitize input used for `WP_Query` (sanitize early, escape late)

-- 
Ticket URL: <https://core.trac.wordpress.org/ticket/60009#comment:1>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform