WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 5 years ago

#10729 closed defect (bug) (fixed)

Potential code injection risk.

Reported by: hakre Owned by: ryan
Milestone: 2.9 Priority: normal
Severity: normal Version: 2.8.4
Component: Security Keywords:
Focuses: Cc:

Description

eval-compareable code injection possible with this code signature:

add_action('admin_notices', create_function( , "echo '$message';" ) );

found in wp-admin/post.php on line ~150.

inject squence "\'; PAYLOAD-PHP-CODE " as $message, done.

code will be executed when admin_notices is fired.

Attachments (2)

10729.patch (706 bytes) - added by hakre 5 years ago.
Security Patch
10729.2.patch (4.7 KB) - added by hakre 5 years ago.
create_function code injection security patch

Download all attachments as: .zip

Change History (17)

comment:1 @tomontoast5 years ago

Full code is:

$message = sprintf( __( 'Warning: %s is currently editing this post' ), esc_html( $last_user_name ) );
149	                $message = str_replace( "'", "\'", "<div class='error'><p>$message</p></div>" );
150	                add_action('admin_notices', create_function( '', "echo '$message';" ) );

so the problem would only occur when the malicious code is in a user's display name.
Nonetheless a very dangerous vulnerability.

comment:2 @hakre5 years ago

and an easy to fix one. I had an idea while getting awake this morning. I will create a patch now.

@hakre5 years ago

Security Patch

comment:3 @hakre5 years ago

  • Milestone changed from Unassigned to 2.9

I added the patch. Code Execution is prevented now because it is not executed any longer but handeled as html-string-data instead (and that's what it is).

I will further analyze the code if there are similar scenarios elsewhere.

I patched against trunk, someone need to open a backport ticket for 2.8.5.

@hakre5 years ago

create_function code injection security patch

comment:4 @hakre5 years ago

I was able to find other places where similar potential code-injections are possible. Most of them are fixed with the last patch, two of them are are marked with FIXME todo tags because I was not able to fix them right now and/or to verify wether or not those are actually dangerous. The function with the two FIXME tags in it is the most potential one so far as I can say.

comment:5 @westi5 years ago

  • Cc westi added

comment:6 @Derevko5 years ago

  • Cc giuseppe@… added

comment:7 @westi5 years ago

Trying to switch as many of these as possible just away from using create_function at all.

comment:8 @westi5 years ago

(In [11923]) Switch the post|page being editing message from a create_function call to a normal function and reduce the duplicated code. See #10729.

comment:9 @westi5 years ago

(In [11925]) Use the old strings which are more translator friendly and add a generic default string to aid re-use by plugins adding post_types. See #10729.

comment:10 @hakre5 years ago

+1 for: "switch as many of these as possible away from using create_function at all"

comment:11 @westi5 years ago

(In [11957]) Move theme preview away from using create_function and to predefined functions. See #10729.

comment:12 @westi5 years ago

(In [12068]) Backport of the switch of the post|page being editing message from a create_function call to a normal function and reduce the duplicated code. See #10729 for 2.8 branch.

comment:13 @automattor5 years ago

(In [12069]) Update version info in phpDoc for _admin_notice_post_locked() see #10729.

comment:14 @westi5 years ago

(In [12070]) Move theme preview away from using create_function and to predefined functions. See #10729 for 2.8 branch.

comment:15 @ryan5 years ago

  • Resolution set to fixed
  • Status changed from new to closed
Note: See TracTickets for help on using tickets.