array_unshift() PHP Warning when wp.getUsersBlogs is attacked (poorly)

[kitchin]

An error_log is getting cluttered with these due to a distributed XML-RPC attack:

PHP Warning: array_unshift() expects parameter 1 to be array, string given in /../wp-includes/class-wp-xmlrpc-server.php on line 518

I tracked it down to this attack, username without password:

<?xml_version] => "1.0" encoding="iso-8859-1"?>
<methodCall><methodName>wp.getUsersBlogs</methodName><params>
<param><value>someuser</value></param>
<param><value></value></param>
</params></methodCall>

What's happening: as in Ticket #16980 untyped empty values are ignored by IRX and the expected 2-element array becomes a singleton. Then the singleton becomes a string (line 455 of class-IXR.php), and hence the array_unshift() warning.

This all ends up at class-wp-xmlrpc-server.php Line 223:

$user = wp_authenticate( 'o', 'm')

using the [1] and [2] elements of the string someuser.

It would be easy to bail at the first line of function wp_getUsersBlogs if $args is not an array. We could issue the same warning as a login failure if we don't want to alert the script kiddies to their errors (that's the current behavior).

There are a few other functions with the same problem, if it is a problem. We could try to use the signature interface of IXR or just do it the easy way.

[jorbin]

@maxcutler or @josephscott - Would one of you be able to take a look here and see what is going on?

[markoheijnen]

I will look into it.

[wonderboymusic]

Anyone is welcome to patch this

[Howdy_McGee]

This has become a big issue recently on my sites. Could we not just verify at the beginning that we're working with an array? Most the issues I've seen in my debug logs is that it's either being passed null or a string instead of the expected array.

[rpayne7264]

I am running into this issue due to distributed XML-RPC attacks, as well.

[rpayne7264]

Replying to wonderboymusic:

Anyone is welcome to patch this

How can I take ownership and work on a patch?

[kitchin]

I see you have commits in plugins. For core WP, my experience has been to checkout trunk or develop in SVN and make a patch. The owner will review it.

Also I have some new notes.

• Like you say, it's an easy fix if we ignore trying to get fancy with the IXR interface - telling it what to expect, etc.

• Might also want to check that trunk still has this bug!! After the current release branch 4.6.x, the IXR classes were relocated from a single file to a directory. I don't know what went into that, or if it was meant to fix anything.

• Feeding test XML into WP is not that hard but I can't recall if it's covered by unit tests - apparently not. That might slow you down if the bug owner asks for tests. I did my testing manually.

• Best practice is to use the develop tree and run command line unit tests on your patched tree.

• If anyone is tracing IXR code, this is where the code was relocated:

from
wp-includes/class-IXR.php Line 455 (in WP 4.0)
to
wp-includes/IXR/class-IXR-server.php Line 103 (in trunk)

It looks like this:

// Perform the callback and send the response
if (count($args) == 1) {
    // If only one parameter just send that instead of the whole array
    $args = $args[0];
}

[kitchin]

Untested patch.

[kitchin]

Maybe you can work with that!

[rpayne7264]

Replying to kitchin:

Maybe you can work with that!

Your patch looks fine, to me, and should be applied, ASAP.

I have addressed my situation by turning off XML RPC completely, by placing the following line in a MU plugin:

add_filter( 'xmlrpc_enabled', '__return_false' );

[rpayne7264]

I just realized that in the case of a call to wp_getUsersBlogs, the xmlrpc_enabled filter will not stop the error that is the subject of this ticket.

[SergeyBiryukov]

In 38600:

XML-RPC: Check the minimum number of arguments in ::wp_getUsersBlogs() and ::blogger_getUsersBlogs().

Props kitchin for initial patch.
Fixes #29750.

[galbaras]

#38300 was marked as a duplicate.