[wp-trac] [WordPress Trac] #40888: PHP Warnings when POSTing keys as arrays to wp-login.php

[WordPress Trac]

#40888: PHP Warnings when POSTing keys as arrays to wp-login.php
------------------------------------+------------------------------
 Reporter:  johnjamesjacoby         |       Owner:
     Type:  defect (bug)            |      Status:  new
 Priority:  normal                  |   Milestone:  Awaiting Review
Component:  Login and Registration  |     Version:
 Severity:  normal                  |  Resolution:
 Keywords:  2nd-opinion             |     Focuses:
------------------------------------+------------------------------

Old description:

> I'm seeing bots filling up my error logs by POSTing to `wp-login.php`
> with `user_login` as an array instead of a string. The `user_login` value
> is blindly passed through functions that assume it's a string, like
> `trim()`, `register_new_user()`, `reset_password()` and so on.
>
> For me, they're hitting `/wp-login.php?action=lostpassword`, but upon
> further review, the majority of actions and functions in (and related to
> `wp-login.php` are equally susceptible to a similar log-filling type of
> attack.
>
> I'm seeing this on PHP7.1, so it's possible that upped the priority to a
> warning which is why I'm seeing this now, but it's also possible this is
> new, or I haven't seen this myself before.
>
> It is possible to setup web-server rules to prevent malformed values in
> these fields, but I think it's better for everyone if `wp-login.php`
> protect against them at the application level anyways.
>
> FWIW, I am not against modifying `$_POST` directly in cases like this
> (where the core code has never supported array values in these keys,
> there's no imaginable reason for these values to ever not be strings, and
> a complex plugin stack means other code probably also trusts these values
> are strings, too.)
>
> Somewhat related: #34192

New description:

 I'm seeing bots filling up my error logs by POSTing to `wp-login.php` with
 `user_login` as an array instead of a string. The `user_login` value is
 blindly passed through functions that assume it's a string, like `trim()`,
 `register_new_user()`, `reset_password()` and so on.

 For me, they're hitting `/wp-login.php?action=lostpassword`, but upon
 further review, the majority of actions and functions in (and related to)
 `wp-login.php` are equally susceptible to a similar log-filling type of
 attack.

 (I'm seeing this on PHP7.1, so it's possible that upped the priority to a
 warning which is why I'm seeing this now, but it's also possible this is
 new, or I haven't seen this myself before.)

 It is possible to setup web-server rules to prevent malformed values in
 these fields, but I think it's better for everyone if `wp-login.php`
 protect against them at the application level anyways.

 FWIW, I am not against modifying `$_POST` directly in cases like this
 (where the core code has never supported array values in these keys,
 there's no imaginable reason for these values to ever not be strings, and
 a complex plugin stack means other code probably also trusts these values
 are strings, too.)

 To duplicate, send the following `$_POST` request to the following URL:

 {{{
 URL: http://src.wordpress-develop.dev/wp-login.php?action=lostpassword

 POST: user_login['test'] => 'hello'
 }}}

 Somewhat related: #34192

--

Comment (by johnjamesjacoby):

 Added a quick duplication step, and edited some grammar and typos.

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