WordPress.org

Make WordPress Core

Opened 4 years ago

Closed 8 months ago

Last modified 8 months ago

#14290 closed defect (bug) (fixed)

register_new_user() can't handle WP_Error result from wp_create_user()

Reported by: coffee2code Owned by: nacin
Milestone: 3.7 Priority: normal
Severity: normal Version: 3.0
Component: Users Keywords: has-patch needs-unit-tests
Focuses: Cc:

Description

register_new_user() in wp-login.php does not properly handle a WP_Error object, which is the potential return value from its call to wp_create_user().

wp_create_user() is basically a pass-through to wp_insert_user(). As a result of [12778], the commit of a patch that was part of #11644 (the MU-merge ticket), new checks were added to wp_insert_user() which changed the potential behavior of the function to return a WP_Error object. register_new_user() can thus fail with a FATAL error:

Notice: Object of class WP_Error could not be converted to int in /Users/scott/Sites/wp30.dev/wp-includes/functions.php on line 3150

Catchable fatal error: Object of class WP_Error could not be converted to string in /Users/scott/Sites/wp30.dev/wp-includes/formatting.php on line 2782

Currently it is not likely the above situation would be tripped. But this is only because register_new_user() and wp_insert_user() perform almost identical error checks (the former does more, actually). However, the errors generated in register_new_user() can all be suppressed by plugins via the 'registration_errors' hook. However, some those same error checks will then be performed in wp_insert_user() and thus return a WP_Error (without a way to suppress those).

Related issues aside, the fact of the matter is that wp_create_user() can return a WP_Error object and register_new_user() can't handle it. The fix is simple and attached.

Attachments (3)

14290.diff (568 bytes) - added by coffee2code 4 years ago.
Patch
14290.2.diff (592 bytes) - added by coffee2code 3 years ago.
Updated patch that accounts for potential 0 return value
14290.3.diff (597 bytes) - added by coffee2code 8 months ago.
Refreshed patch to apply cleanly as of r25172.

Download all attachments as: .zip

Change History (10)

coffee2code4 years ago

Patch

comment:1 coffee2code4 years ago

Also see the related #14308 (and my first comment there). Any consensus made there might relate to whether this particular issue becomes moot or not.

comment:3 mdawaffe3 years ago

  • Milestone changed from Awaiting Review to Future Release

Looks like wp_insert_user() could in theory still return 0, so the check should be for !$user_id || is_wp_error( $user_id ).

comment:4 coffee2code3 years ago

@mdawaffe is correct, though it looks to be pretty rare that a 0 would be returned by wp_insert_user(). Updated patch has been attached (14290.2.diff), which refreshes original patch against latest trunk and accounts for $user_id potentially being 0 or a WP_Error. But to be clear, wp_insert_user() is meant to return a WP_Error if there is an error and register_new_user() can't handle it without this patch.

coffee2code3 years ago

Updated patch that accounts for potential 0 return value

coffee2code8 months ago

Refreshed patch to apply cleanly as of r25172.

comment:5 coffee2code8 months ago

  • Milestone changed from Future Release to 3.7

Simple and obvious fix for 3.7? Especially given #15001.

comment:6 nacin8 months ago

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

In 25174:

Check for a WP_Error return from wp_create_user() in register_new_user().

props coffee2code.
fixes #14290.

comment:7 nacin8 months ago

  • Keywords needs-unit-tests added

This seems like it could be unit testable?

Note: See TracTickets for help on using tickets.