Opened 9 months ago
Last modified 3 months ago
#61125 reviewing enhancement
Many strings or URLs lack proper escaping.
Reported by: | yagniksangani | Owned by: | 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)
Change History (5)
#1
@
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
@
8 months ago
#58305 already suggested escaping $login_header_text
, but the decision there was to leave that variable unescaped.
#3
@
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 (inwp-links-opml.php
). - When I edited existing links, with the Link Manager plugin activated, the
updated
value remained blank space. However, ifwp-links-opml.php
ever prints something that needs escaping in that attribute, it should useecho esc_attr( $bookmark->link_updated );
instead ofesc_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.
Added escaping for some Strings and URLs