WordPress.org

Make WordPress Core

Opened 3 years ago

Closed 3 years ago

#34336 closed defect (bug) (fixed)

Disable XML-RPC system.multicall authenticated requests on the first auth failure

Reported by: dd32 Owned by: dd32
Milestone: 4.4 Priority: normal
Severity: normal Version:
Component: XML-RPC Keywords:
Focuses: Cc:

Description

Recently Securi published a post about a Brute Force Amplification Attack affecting WordPress by using system.multicall.

WordPress should cause XML-RPC authentication to fail on all subsequent multicall calls silently to prevent this attack being viable against WordPress.

The attached patch implements this suggestion, and although it breaks the XML-RPC spec I think we should enforce this.
Multiple user authentications are still possible when using system.multicall, the only catch is that once one fails authentication, all the further attempts will also fail.

Attachments (2)

34336.diff (2.5 KB) - added by dd32 3 years ago.
34336.tests.diff (1.7 KB) - added by johnbillion 3 years ago.

Download all attachments as: .zip

Change History (10)

@dd32
3 years ago

#1 @johnbillion
3 years ago

  • Keywords has-unit-tests added; needs-testing removed

#2 @Otto42
3 years ago

+1. This is a simple patch that does not break anything, except perhaps the spec. Given the planned changes for REST, this is a good mitigation.

#3 @dd32
3 years ago

  • Owner set to dd32
  • Resolution set to fixed
  • Status changed from new to closed

In 35366:

XMLRPC: Prevent authentication from occuring after a failed authentication attmept in any single XML-RPC call.

This hardens WordPress against a common vector which uses multiple user identifiers in a single system.multicall call. In the event that authentication fails, all following authentication attempts in that call will also fail.

Props dd32, johnbillion.
Fixes #34336

#4 @dd32
3 years ago

In 35367:

XMLRPC: Revert the changes to WP_XMLRPC_UnitTestCase in [35366] as they weren't required.

See #34336

#5 @dd32
3 years ago

Thanks for the Unit Tests @johnbillion.

I had to split up test_login_pass_ok to test failing authentication separately as it was relying upon the changed behaviour.

#6 follow-up: @knutsp
3 years ago

A candidate for other branches?

#7 in reply to: ↑ 6 @johnbillion
3 years ago

  • Keywords fixed-major added; has-patch has-unit-tests removed
  • Milestone changed from 4.4 to 4.3.2
  • Resolution fixed deleted
  • Status changed from closed to reopened

Replying to knutsp:

A candidate for other branches?

Yep, if a 4.3.2 happens.

#8 @ocean90
3 years ago

  • Keywords fixed-major removed
  • Milestone changed from 4.3.2 to 4.4
  • Resolution set to fixed
  • Status changed from reopened to closed
Note: See TracTickets for help on using tickets.