WordPress.org

Make WordPress Core

Opened 6 years ago

Closed 5 years ago

#10896 closed defect (bug) (fixed)

preg_replace with eval modifier used in _fix_attachment_links

Reported by: westi Owned by: westi
Milestone: 2.9 Priority: high
Severity: major Version: 2.8.4
Component: Security Keywords: needs-patch
Focuses: Cc:

Description

Reported by BenBE1987 on #8689

This code:

		$post_search[$i] = $anchor;
		$post_replace[$i] = preg_replace( "#href=(\"|')[^'\"]*\\1#e", "stripslashes( 'href=\\1' ).get_attachment_link( $id ).stripslashes( '\\1' )", $anchor );
		++$i;

Change History (5)

comment:1 @BenBE19876 years ago

Patched locally for me as:

//
// Private
//

global $_fix_attachment_link_id;
function _fix_attachment_links_replaceCB($match) {
        global $_fix_attachment_link_id;
        return stripslashes( 'href='.$m[1] ).get_attachment_link( $_fix_attachment_link_id ).stripslashes( $m[1] );
}

/**
 * Replace hrefs of attachment anchors with up-to-date permalinks.
 *
 * @since unknown
 * @access private
 *
 * @param unknown_type $post_ID
 * @return unknown
 */
function _fix_attachment_links( $post_ID ) {
        global $_fix_attachment_link_id;

        $post = & get_post( $post_ID, ARRAY_A );

        $search = "#<a[^>]+rel=('|\")[^'\"]*attachment[^>]*>#ie";

        // See if we have any rel="attachment" links
        if ( 0 == preg_match_all( $search, $post['post_content'], $anchor_matches, PREG_PATTERN_ORDER ) )
                return;

        $i = 0;
        $search = "#[\s]+rel=(\"|')(.*?)wp-att-(\d+)\\1#i";
        foreach ( $anchor_matches[0] as $anchor ) {
                if ( 0 == preg_match( $search, $anchor, $id_matches ) )
                        continue;

                $id = (int) $id_matches[3];

                // While we have the attachment ID, let's adopt any orphans.
                $attachment = & get_post( $id, ARRAY_A );
                if ( ! empty( $attachment) && ! is_object( get_post( $attachment['post_parent'] ) ) ) {
                        $attachment['post_parent'] = $post_ID;
                        // Escape data pulled from DB.
                        $attachment = add_magic_quotes( $attachment);
                        wp_update_post( $attachment);
                }

                $post_search[$i] = $anchor;
                $_fix_attachment_link_id = $id;
                $post_replace[$i] = preg_replace_callback( "#href=(\"|')[^'\"]*\\1#", '_fix_attachment_links_replaceCB', $anchor );
                ++$i;
        }

        $post['post_content'] = str_replace( $post_search, $post_replace, $post['post_content'] );

        // Escape data pulled from DB.
        $post = add_magic_quotes( $post);

        return wp_update_post( $post);
}

Not sure if this fully works.

comment:2 @BenBE19876 years ago

  • Milestone changed from 2.9 to 2.8.5
  • Priority changed from normal to high
  • Severity changed from normal to major

comment:3 @westi6 years ago

  • Milestone changed from 2.8.5 to 2.9

comment:4 @hakre6 years ago

I think the original author of that function should take care and remove it by her/himself. Really that needs to go out. And please add a policy to the coding guidelines that these eval switches are not to be used any longer.

comment:5 @ryan5 years ago

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

(In [12198]) Use preg_replace_callback to eliminate eval. Props BenBE1987. fixes #10896

Note: See TracTickets for help on using tickets.