Proposal to check for is_scalar() in WP_Query::fill_query_vars()

[tivnet]

The Symptom:

PHP Warning: strlen() expects parameter 1 to be string, array given in .../wp-includes/query.php on line 1577

The line:

if ( ! empty( $qv['s'] ) && strlen( $qv['s'] ) > 1600 )

The cause (hacker's script):

http://www.example.com/?s[]=something

The 's' becomes an array, causing a sequence of warnings

My quick patch in the core:

if ( ! is_scalar( $array['s'] ) ) {
	$array['s'] = '';
}

A patch outside the core, until there is a upgrade:

/** Hooked to 'parse_request' */
function action_parse_request(WP $query_args) {
	if ( ! is_scalar( $query_args->query_vars['s'] ) ) {
		$query_args->query_vars['s'] = '';
	}
}

The proposal:

To check all (or most of) variables in the fill_query_vars()'s first loop for is_scalar()

[tivnet]

Of course, isset or ! empty should be checked before is_scalar, for example:

if ( isset( $query_args->query_vars['s'] ) && ! is_scalar( $query_args->query_vars['s'] ) ) {
        $query_args->query_vars['s'] = '';
}

[boonebgorges]

Yes, the change looks good. 29736.patch​ makes it more concise and adds unit tests.

Of course, isset or ! empty should be checked before is_scalar,

This shouldn't be necessary. By the time we've gotten to this point in parse_query(), the query vars have either been run through fill_query_vars(), which guarantees that 's' will be isset(). An equivalent ! empty() check is done later on: https://core.trac.wordpress.org/browser/tags/4.0/src/wp-includes/query.php#L2655

[boonebgorges]

In 29912:

Check that search value is scalar before parsing.

Prevents PHP notices when non-scalar values are passed.

Includes unit tests.

Props tivnet.
Fixes #29736.

[SergeyBiryukov]

Related: #17737

[swissspidy]

For some reason the condition added in [29912] does not work on HHVM as the tests fail because of a strlen() expects parameter 1 to be string, array given error