Make WordPress Core

Opened 2 months ago

Last modified 14 hours ago

#62436 new defect (bug)

Add proper escaping for dynamic values in login template

Reported by: im3dabasia1's profile im3dabasia1 Owned by:
Milestone: 6.8 Priority: normal
Severity: normal Version:
Component: General Keywords: has-patch close 2nd-opinion
Focuses: coding-standards Cc:

Description

The dynamic values $title and $login_header_text in the login template are not properly escaped.

File: wp-login.php
Line Number: 217, and 222

Change History (6)

This ticket was mentioned in PR #7809 on WordPress/wordpress-develop by @im3dabasia1.


2 months ago
#1

  • Keywords has-patch added

Trac ticket: https://core.trac.wordpress.org/ticket/62436

### PR Description
Applied proper escaping to dynamic values in the login template:

  • esc_html() for $title to safely render any special characters in the text.
  • esc_html() for $login_header_text to ensure safe output of the login header text.

These changes enhance security by preventing potential XSS vulnerabilities.

@narenin commented on PR #7809:


2 months ago
#2

@im3dabasia Good catch!
The PR looks good to me.

This ticket was mentioned in Slack in #core by desrosj. View the logs.


2 months ago

#4 @desrosj
2 months ago

  • Component changed from Administration to General
  • Focuses coding-standards added
  • Milestone changed from Awaiting Review to 6.8

#5 @sabernhardt
2 months ago

#58305 (plus #59257 and #61125) already proposed escaping $login_header_text with esc_html(), and the ticket was closed. If this uses wp_kses() instead, it could allow img elements because some sites might remove the text-indent to have an image instead of the background.

The $title variable stands for the first parameter of the login_header() function ('Log In' by default). I did not think escaping the variable was necessary with the Core translatable strings, but some plugins use login_header() too. A few of them, such as Google Authenticator, even have esc_html__() in the parameter.

Last edited 6 weeks ago by sabernhardt (previous) (diff)

#6 @audrasjb
14 hours ago

  • Keywords close 2nd-opinion added

Yes, I concur with @sabernhardt's feedback: both strings have very broadly used filters, and stripping HTML –even with the proposed KSES implementation– from them would most certainly break a lot of plugins.

My opinion is to close this ticket as wontfix.

Note: See TracTickets for help on using tickets.