WordPress.org

Make WordPress Core

Opened 4 years ago

Closed 4 years ago

#16058 closed defect (bug) (fixed)

Error massage HTML tags escaped

Reported by: mako09 Owned by:
Milestone: 3.1 Priority: normal
Severity: normal Version:
Component: General Keywords: has-patch commit
Focuses: Cc:

Description

esc_html() is used in the line 821 of wp-admin/includes/file.php. But the default message defined in 3 lines above has HTML tags.

        if ( $error ) {
                $error_string = __('<strong>Error:</strong> There was an error connecting to the server, Please verify the settings are correct.');
                if ( is_wp_error($error) )
                        $error_string = $error->get_error_message();
                echo '<div id="message" class="error"><p>' . esc_html( $error_string ) . '</p></div>';
        }

Attachments (2)

16058.patch (629 bytes) - added by SergeyBiryukov 4 years ago.
16058.2.patch (708 bytes) - added by SergeyBiryukov 4 years ago.

Download all attachments as: .zip

Change History (12)

@SergeyBiryukov4 years ago

comment:1 @SergeyBiryukov4 years ago

  • Keywords has-patch added

Since we don't seem to escape error messages in other places, I suppose we don't have to do it here too.

comment:2 @nacin4 years ago

  • Keywords needs-patch added; has-patch removed
  • Milestone changed from Awaiting Review to 3.1

We need to here. It's possible to XSS yourself otherwise (I added this in 3.0.2.) We can just move the esc_html up a line.

@SergeyBiryukov4 years ago

comment:3 @SergeyBiryukov4 years ago

  • Keywords has-patch added; needs-patch removed

Done. Couldn't yet find a description for this specific change. What about other places where get_error_message() is used, like the first instance in wp-admin/includes/media.php?

comment:4 @nacin4 years ago

Only here. The username and password get sent on a long and winding path through wp-admin/includes. You might end up with this string as the error message: Username/Password incorrect for %s, where %s is the username as direct input. The area is nonced and guarded by super admin credentials, though, so it was a minor fix.

comment:5 @SergeyBiryukov4 years ago

Got it, thanks.

comment:6 @nacin4 years ago

  • Keywords commit added

comment:7 @nacin4 years ago

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

(In [17200]) Don't esc_html the default error string. props SergeyBiryukov, fixes #16058.

comment:8 @anthonycole4 years ago

  • Cc anthonycole added
  • Resolution fixed deleted
  • Status changed from closed to reopened

I'm getting this escaped error message with an incorrect password...

<strong>Error:</strong> There was an error connecting to the server, Please verify the settings are correct.

comment:9 @nacin4 years ago

In 3.1?

comment:10 @dd324 years ago

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

I've spoken with Anthony, and I believe this was a old version of a file.

Note: See TracTickets for help on using tickets.