Make WordPress Core

Opened 9 months ago

Last modified 3 months ago

#61125 reviewing enhancement

Many strings or URLs lack proper escaping.

Reported by: yagniksangani's profile yagniksangani Owned by: audrasjb's profile audrasjb
Milestone: Awaiting Review Priority: normal
Severity: minor Version:
Component: General Keywords: has-patch
Focuses: coding-standards Cc:

Description

Upon reviewing various strings and URLs within the WordPress core codebase, it has been noted that many instances lack proper escaping.

This absence of escaping poses a security risk, as it can potentially lead to vulnerabilities such as cross-site scripting (XSS) attacks. Unescaped output allows malicious users to inject scripts into web pages viewed by other users.

For example, you can see it here, \wp-activate.php

Attachments (2)

some-feature.61125.diff (3.1 KB) - added by yagniksangani 9 months ago.
Added escaping for some Strings and URLs
61125.login_title.diff (504 bytes) - added by sabernhardt 3 months ago.
adds escaping for filterable $login_title (only)

Download all attachments as: .zip

Change History (5)

@yagniksangani
9 months ago

Added escaping for some Strings and URLs

#1 @audrasjb
8 months ago

  • Keywords changes-requested added
  • Owner set to audrasjb
  • Severity changed from major to minor
  • Status changed from new to reviewing

Hello and thanks for the ticket and patch,

However, esc_html_e is a internationalization function and should not be used here as this function is only meant for translatable text.

#2 @sabernhardt
8 months ago

#58305 already suggested escaping $login_header_text, but the decision there was to leave that variable unescaped.

@sabernhardt
3 months ago

adds escaping for filterable $login_title (only)

#3 @sabernhardt
3 months ago

  • Keywords changes-requested removed

I agree to escape the filtered title tag contents from $login_title.

Regarding other changes proposed in some-feature.61125.diff:

  • [57625] already addressed escaping output in wp-activate.php.
  • I do not think the gmdate() function needs any escaping (in wp-links-opml.php).
  • When I edited existing links, with the Link Manager plugin activated, the updated value remained blank space. However, if wp-links-opml.php ever prints something that needs escaping in that attribute, it should use echo esc_attr( $bookmark->link_updated ); instead of esc_html.
  • As mentioned in comment:2, #58305 purposely did not escape $login_header_text.

I also had planned to add esc_html for the $title variable in the new visually hidden login heading, but I did not find a need to escape the translatable strings.

Note: See TracTickets for help on using tickets.