Make WordPress Core

Opened 19 months ago

Closed 6 months ago

Last modified 6 months ago

#56558 closed defect (bug) (fixed)

Fatal Error caused by Uncaught TypeError: trim(): Argument #1 ($string) must be of type string, array given in /www/example_website/public/wp-includes/class-wp-query.php:803

Reported by: rlmc's profile rlmc Owned by: sergeybiryukov's profile SergeyBiryukov
Milestone: 6.4 Priority: normal
Severity: minor Version: 1.5
Component: Query Keywords: needs-testing has-patch needs-testing-info
Focuses: Cc:

Description (last modified by SergeyBiryukov)

I found this in my error.log file on my web server. If I paste the same url onto my website, it does indeed cause a fatal error. Here is the full entry from the error.log file...the only change I made is to disguise my website as example.com:

2022/09/11 01:49:53 [error] 90419#90419: *4359495 FastCGI sent in stderr: "PHP message: PHP Fatal error:  
Uncaught TypeError: trim(): Argument #1 ($string) must be of type string, array given in /www/example_website/public/wp-includes/class-wp-query.php:803

Stack trace:
#0 /www/example_website/public/wp-includes/class-wp-query.php(803): trim(Array)
#1 /www/example_website/public/wp-includes/class-wp-query.php(1792): WP_Query->parse_query()
#2 /www/example_website/public/wp-includes/class-wp-query.php(3586): WP_Query->get_posts()
#3 /www/example_website/public/wp-includes/class-wp.php(648): WP_Query->query(Array)
#4 /www/example_website/public/wp-includes/class-wp.php(775): WP->query_posts()
#5 /www/example_website/public/wp-includes/functions.php(1330): WP->main('')
#6 /www/example_website/public/wp-blog-header.php(16): wp()
#7 /www/example_website/public/index.php(17): require('/www/example...')
#8 {main}
  thrown in /www/examplewebsite" while reading response header from upstream, client: 82.199.155.149, server: www.example.com, 
request: "POST /example_page?q=user%2Fpassword&name%5B%23post_render%5D%5B%5D=passthru&name%5B%23type%5D=markup&name%5B%23markup%5D=id HTTP/1.1", 
upstream: "fastcgi://unix:/var/run/php8.0-fpm-examplewebsitehhub.sock:", host: "www.example.com:61359"

Change History (21)

#1 @desrosj
18 months ago

  • Version trunk deleted

#2 @brookedot
9 months ago

This recently came up in wp-includes/class-wp-query.php:1973 on a site I was working on. As the site was running WordPress 6.1, I am linking to the same line in 6.2 but the line number has changed
https://github.com/WordPress/wordpress-develop/blob/6.2/src/wp-includes/class-wp-query.php#L2011-L2014

The URL passed looks like this:
www.example.com.com/tags/the-tage/?page[%24eq]=1

In this case, some bad actor seems to be trying to brute-force the URL. It appears the solution may be to add some additional validation on the page query to return if anything but a string on the trim.

Looking at the original report though, it seems there may be several places where adjustments need to be made.

Thoughts?

#3 @SergeyBiryukov
9 months ago

  • Description modified (diff)
  • Keywords needs-patch added
  • Milestone changed from Awaiting Review to 6.4

Hi there, thanks for the report!

I believe the original issue has been fixed in [53891] / #17737.

It looks like $q['page'] was not a part of that commit though, and needs a similar fix as per comment:2.

#4 @brookedot
9 months ago

  • Keywords needs-testing has-patch added; needs-patch removed

Thanks for pointing me to the PR which fixes the original portion of this ticket @SergeyBiryukov!

I have created a PR over at GitHub to fix the $page portion, this is my first time doing a PR in GitHub instead of via SVN so let me know if I did it right.

https://github.com/WordPress/wordpress-develop/pull/4920

This ticket was mentioned in PR #4920 on WordPress/wordpress-develop by BrookeDot.


9 months ago
#5

This PR builds on the work completed with the closure of Trac-17737

It ensures that the value of $page in a query is scalar prior to attempting to trim or assign it to an absint avoiding a fatal error if an array is passed.

Fixes: (comment 2)

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

Edit: update first Trac link as the PR was associated with the wrong trac ticket.

#6 @brookedot
9 months ago

  • Severity changed from major to minor
  • Version set to 1.5

@SergeyBiryukov commented on PR #4920:


9 months ago
#7

Thanks for the PR! Left a minor comment, looks good to me otherwise 🙂

BrookeDot commented on PR #4920:


9 months ago
#8

Thanks for the PR! Left a minor comment, looks good to me otherwise 🙂

Yep, I did briefly think about that ;)

This ticket was mentioned in Slack in #core by nicolefurlan. View the logs.


7 months ago

#10 @nicolefurlan
7 months ago

  • Keywords needs-testing-info added

#11 @nicolefurlan
7 months ago

@rlmc could you add testing instructions to this ticket?

This ticket was mentioned in Slack in #core by oglekler. View the logs.


6 months ago

@mukesh27 commented on PR #4920:


6 months ago
#13

@BrookeDot Could you please rebase your PR?

BrookeDot commented on PR #4920:


6 months ago
#14

Could you please rebase your PR?

@mukeshpanchal27 against trunk right?

@mukesh27 commented on PR #4920:


6 months ago
#15

Yes

BrookeDot commented on PR #4920:


6 months ago
#16

I think I did it @mukeshpanchal27 I see the tests are running now.

Let me know, I'm new to contributing via GitHub :D

BrookeDot commented on PR #4920:


6 months ago
#17

Looks like one test failed, unrelated to the PR. Can you run it again?

@mukesh27 commented on PR #4920:


6 months ago
#18

Thanks @BrookeDot, For the https://github.com/WordPress/wordpress-develop/pull/4920#discussion_r1346856772 i would like to get @SergeyBiryukov thought.

#19 @SergeyBiryukov
6 months ago

  • Owner set to SergeyBiryukov
  • Status changed from new to reviewing

#20 @SergeyBiryukov
6 months ago

  • Resolution set to fixed
  • Status changed from reviewing to closed

In 56815:

Query: Ensure that the page parameter is scalar in WP_Query::get_posts().

The page query var only accepts a scalar value and passes the value through functions that assume a scalar value.

Adding an extra guard condition does not affect its functionality but does avoid a PHP fatal error for trim() when a non-scalar value such as an array is passed.

Follow-up to [2535], [53891].

Props brookedot, rlmc, mukesh27, SergeyBiryukov.
Fixes #56558.

@SergeyBiryukov commented on PR #4920:


6 months ago
#21

Thanks for the PR! Merged in r56815.

Note: See TracTickets for help on using tickets.