Make WordPress Core

Opened 20 months ago

Last modified 3 months ago

#62828 new defect (bug)

Array should not be passed to get_page_by_path()

Reported by: leedxw Owned by:
Priority: normal Milestone: Awaiting Review
Component: General Version: 6.7.2
Severity: normal Keywords: has-patch
Cc: Focuses:

Description

We have been logging the following, as a result of an unauthorised vulnerability scan on a site we host:

PHP Warning:  urldecode() expects parameter 1 to be string, array given in /var/www/html/wp-includes/post.php on line 6033

This occurs when an array is being passed to get_page_by_path()

I was able to reproduce using

curl -g "http://localhost/?year[1]=1"

This specific instance occurred in wp_resolve_numeric_slug_conflicts() where user-supplied values of "month" "year" etc, are passed to get_page_by_path()

Attachments (2)

rewrite_path.patch (520 bytes ) - added by leedxw 20 months ago.
Patch to rewrite.php to check for array
rewrite_672.patch (1.3 KB ) - added by leedxw 17 months ago.
Patch for 62828 for 6.7.2

Download all attachments as: .zip

Change History (13)

@leedxw
20 months ago

Patch to rewrite.php to check for array

#1 @sukhendu2002
20 months ago

  • Keywords has-patch added
  • Version6.7.1

Reproduction Report

Description

This report validates whether the issue can be reproduced.

Environment

  • WordPress: 6.7.1
  • PHP: 8.2.27
  • Server: nginx/1.27.3
  • Database: mysqli (Server: 8.0.40 / Client: mysqlnd 8.2.27)
  • Browser: Chrome 131.0.0.0
  • OS: macOS
  • Theme: Twenty Twenty-Five 1.0
  • MU Plugins: None activated
  • Plugins:
    • Test Reports 1.2.0

Actual Results

  1. ✅ Error condition occurs (reproduced).

Additional Notes

I am able to reproduce the issue in WordPress 6.7.1; however, I am not able to reproduce it in the trunk.

#2 @sainathpoojary
20 months ago

I can confirm that the "Warning: Array to string conversion" warning is occurring in WordPress 6.7.1, when an array is passed to get_page_by_path(). However, this issue does not happen in the current trunk version.

WordPress 6.7.1:
https://rioudcpuyg.ufs.sh/f/PL8E4NiPUWyOM9i9B5hp3F90beESqliHBLy7PKXG5nvmwjAd

Trunk:
https://rioudcpuyg.ufs.sh/f/PL8E4NiPUWyO6qsDFx8XILecyvHwBdbisfSAoq2h5TGkQU0C

#3 @abcd95
20 months ago

Hey @leedxw, Thanks for bringing this up.

As mentioned in the above two comments, the warning is visible in 6.7.1 and not on the trunk. So I upgraded to the nightly build, and I was able to see the warning there too.

And the patch works fine fixing the warning. However, it would be better to further sanitize the variable to make sure it is a string passed.

#4 @leedxw
17 months ago

This issue still occurs in 6.7.2

#5 @leedxw
17 months ago

  • Version 6.7.16.7.2
PHP Warning:  Array to string conversion in /var/www/html/wp-includes/post.php on line 6021
PHP Fatal error:  Uncaught TypeError: urldecode(): Argument #1 ($string) must be of type string, array given in /var/www/html/wp-includes/post.php:6033

Stack trace:
#0 /var/www/html/wp-includes/post.php(6033): urldecode()
#1 /var/www/html/wp-includes/rewrite.php(419): get_page_by_path()
#2 /var/www/html/wp-includes/class-wp.php(390): wp_resolve_numeric_slug_conflicts() 
#3 /var/www/html/wp-includes/class-wp.php(813): WP->parse_request()
#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('...')
#7 {main}

  thrown in /var/www/html/wp-includes/post.php on line 6033

request: "GET /some-slug/?year%5B0%5D=2022

@leedxw
17 months ago

Patch for 62828 for 6.7.2

#6 @dd32
17 months ago

A simpler fix might be to do this instead, as it'll avoid making the existing selection branch more complex.

        // This is the potentially clashing slug.
        $value = '';
-       if ( $compare && array_key_exists( $compare, $query_vars ) ) {
+       if ( $compare && array_key_exists( $compare, $query_vars ) && is_scalar( $query_vars[ $compare ] ) ) {
                $value = $query_vars[ $compare ];
        }
Version 0, edited 17 months ago by dd32 (next)

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


17 months ago
#7

This avoids invalid input from causing PHP Warnings & Fatals, such as when the $compare query field contains an array (which is not valid).

This also removes the duplicate check of $compare being set to a value.

Fixes https://core.trac.wordpress.org/ticket/62828

Trac ticket:

#8 @leedxw
17 months ago

Just noting that this currently will produce a fatal error under php8.3 and WordPress 6.8

$ wp option set permalink_structure '/%postname%/'
$ wp rewrite flush
$ curl -s -o /dev/null -w "%{http_code}\n" "http://localhost/hello-world/?year%5B%5D=2"
500

#9 @leedxw
7 months ago

Fatal errors can still be generated with WordPress 6.9 and PHP 8.3

$ wp option set permalink_structure '/%postname%/'
$ wp rewrite flush
$ curl -s -o /dev/null -w "%{http_code}\n" "http://localhost/hello-world/?year%5B%5D=2"
500
Last edited 7 months ago by leedxw (previous) (diff)

#10 @leedxw
4 months ago

Fatal errors can still be generated with WordPress 7.0-RC4-62379 and PHP 8.5

$ wp option set permalink_structure '/%postname%/'
$ wp rewrite flush
$ curl -s -o /dev/null -w "%{http_code}\n" "http://localhost/hello-world/?year%5B%5D=2"
500

#11 @leedxw
3 months ago

Fatal errors can still be generated with WordPress 7.0 and PHP 8.5 - but I can't select 7.0 as a version?

$ wp option set permalink_structure '/%postname%/'
$ wp rewrite flush
$ curl -s -o /dev/null -w "%{http_code}\n" "http://localhost/hello-world/?year%5B%5D=2"
500
Last edited 3 months ago by leedxw (previous) (diff)
Note: See TracTickets for help on using tickets.