Make WordPress Core

Opened 6 weeks ago

Last modified 11 days ago

#60745 new defect (bug)

WP_Query::parse_query() does not handle invalid query arg values

Reported by: xknown's profile xknown Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version:
Component: Query Keywords: has-patch has-unit-tests php80
Focuses: Cc:

Description

The code in WP_Query::parse_query assumes that most query arguments have the correct type. However, this doesn't seem to be the case for the following arguments:

Things that expect arrays, but the code that call them pass (unintended) invalid data types

					'author__in'          => 'string',
					'author__not_in'      => 'string',
					'category__and'       => 'string',
					'category__in'        => 'string',
					'category__not_in'    => 'string',
					'post__in'            => 'string',
					'post__not_in'        => 'string',
					'post_name__in'       => 'string',
					'post_parent__in'     => 'string',
					'post_parent__not_in' => 'string',
					'tag__and'            => 'string',
					'tag__in'             => 'string',
					'tag__not_in'         => 'string',
					'tag_slug__and'       => 'string',
					'tag_slug__in'        => 'string',

Using most of the above query args in a WP_Query::get_posts() call result in a PHP fatal.

Things that expect scalars:

					'attachment'      => array(),
					'author_name'     => array(),
					'feed'            => array(),

Using any of the above query args result in a PHP fatal on a default WP installation:

alex@wayra core % cat .wp-env.json
{
	"core": null
}

http://localhost:8888/?attachment[]=admin

Fatal error: Uncaught TypeError: urlencode(): Argument #1 ($string) must be of type string, array given in /var/www/html/wp-includes/formatting.php:5683 Stack trace: #0 /var/www/html/wp-includes/formatting.php(5683): urlencode(Array) #1 /var/www/html/wp-includes/class-wp-query.php(2183): wp_basename(Array) #2 /var/www/html/wp-includes/class-wp-query.php(3824): WP_Query->get_posts() #3 /var/www/html/wp-includes/class-wp.php(696): WP_Query->query(Array) #4 /var/www/html/wp-includes/class-wp.php(816): WP->query_posts() #5 /var/www/html/wp-includes/functions.php(1336): WP->main('') #6 /var/www/html/wp-blog-header.php(16): wp() #7 /var/www/html/index.php(17): require('/var/www/html/w...') #8 {main} thrown in /var/www/html/wp-includes/formatting.php on line 5683

http://localhost:8888/?author_name[]=admin

Fatal error: Uncaught TypeError: str_contains(): Argument #1 ($haystack) must be of type string, array given in /var/www/html/wp-includes/class-wp-query.php:2358 Stack trace: #0 /var/www/html/wp-includes/class-wp-query.php(2358): str_contains(Array, '/') #1 /var/www/html/wp-includes/class-wp-query.php(3824): WP_Query->get_posts() #2 /var/www/html/wp-includes/class-wp.php(696): WP_Query->query(Array) #3 /var/www/html/wp-includes/class-wp.php(816): WP->query_posts() #4 /var/www/html/wp-includes/functions.php(1336): WP->main('') #5 /var/www/html/wp-blog-header.php(16): wp() #6 /var/www/html/index.php(17): require('/var/www/html/w...') #7 {main} thrown in /var/www/html/wp-includes/class-wp-query.php on line 2358

http://localhost:8888/?feed[]=admin

Fatal error: Uncaught TypeError: str_contains(): Argument #1 ($haystack) must be of type string, array given in /var/www/html/wp-includes/class-wp-query.php:1018 Stack trace: #0 /var/www/html/wp-includes/class-wp-query.php(1018): str_contains(Array, 'comments-') #1 /var/www/html/wp-includes/class-wp-query.php(1868): WP_Query->parse_query() #2 /var/www/html/wp-includes/class-wp-query.php(3824): WP_Query->get_posts() #3 /var/www/html/wp-includes/class-wp.php(696): WP_Query->query(Array) #4 /var/www/html/wp-includes/class-wp.php(816): WP->query_posts() #5 /var/www/html/wp-includes/functions.php(1336): WP->main('') #6 /var/www/html/wp-blog-header.php(16): wp() #7 /var/www/html/index.php(17): require('/var/www/html/w...') #8 {main} thrown in /var/www/html/wp-includes/class-wp-query.php on line 1018

Change History (4)

This ticket was mentioned in PR #6246 on WordPress/wordpress-develop by @xknown.


6 weeks ago
#1

  • Keywords has-patch has-unit-tests added

Add is_scalar/is_array checks.

Trac ticket: https://core.trac.wordpress.org/ticket/60745

#2 @xknown
2 weeks ago

  • Keywords php80 added

#3 @jrf
2 weeks ago

@xknown PHP errors like these exist for a reason: to warn developers and allow them to fix their code.

Hiding these errors as per your proposal (and as previously clearly applied in part as well), is often the cause of insidious bugs and makes debugging faulty queries much harder as some "mystery default" gets used instead of the query throwing an error which the developer can fix.

#4 @peterwilsoncc
11 days ago

I agree with @jrf that these shouldn't be fixed in WP_Query.

For the use cases provided of visitors using the incorrect data type in URLs, eg localhost?attachment[]=foobar, data sanitization should take place in WP::parse_request() before calling WP_Query. The former being a user API, the latter a developer API.

Related #56311.

Note: See TracTickets for help on using tickets.