Opened 10 months ago
Last modified 3 days ago
#57336 assigned defect (bug)
Sanitize url and title missing
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 6.4 | Priority: | normal |
Severity: | major | Version: | |
Component: | Networks and Sites | Keywords: | has-patch needs-testing |
Focuses: | multisite | Cc: |
Description
- The login title in wp-login.php is not sanitized properly https://prnt.sc/MJLEeeUWf7BE
- Network site URL is also not properly sanitized https://prnt.sc/6rFVD0ClxbO-
Change History (10)
This ticket was mentioned in PR #3771 on WordPress/wordpress-develop by vairafiq.
10 months ago
#1
#2
@
7 weeks ago
- Keywords needs-dev-note added
Following on the feedback of the comment here: https://github.com/WordPress/wordpress-develop/pull/3771#discussion_r1050136612
Is it good to be closed @costdev?
#3
@
7 weeks ago
- Keywords 2nd-opinion added; needs-dev-note removed
Hi @rajinsharwar, thanks for the ping! I believe this should remain open as the PR covers some instances in this file, but not all. I'd also like to check out some of the history in this file and see why escaping isn't there already, as there may be valid reasons/decisions made about this.
#4
@
6 weeks ago
- Milestone changed from Awaiting Review to 6.4
- Owner changed from rafiq91 to rajinsharwar
Hi @costdev, I have checked the historical data of those lines but didn't find any strong reason for not escaping. Here are the references:
https://core.trac.wordpress.org/changeset/44021#file0
https://core.trac.wordpress.org/changeset/12605/trunk/wp-activate.php
https://core.trac.wordpress.org/ticket/11644
Taking ownership of the ticket, and placing this for 6.4
This ticket was mentioned in PR #5046 on WordPress/wordpress-develop by @rajinsharwar.
6 weeks ago
#5
Escaping the URLs in the network_site_url function in the file of wp-activate.php
Trac ticket: https://core.trac.wordpress.org/ticket/57336
#6
@
6 weeks ago
- Component changed from Login and Registration to Networks and Sites
- Focuses multisite added; privacy removed
- Keywords 2nd-opinion removed
Thanks @rajinsharwar!
Indeed, escaping was intended - see the second item in this comment - but looks like it wasn't added.
Most other uses of network_site_url()
are escaped in Core, and I don't see why these cases would be an exception.
I've left a comment on PR 5046 about some additional URL escaping needed, and whether we should expand the scope of this ticket and PR to handle other escaping in this file.
- Updating the component to
Networks and Sites
and addingmultisite
focus, as this file is specifically for Multisite. - Removing
privacy
as the proposed change does not affect privacy.
#7
@
6 weeks ago
Great thanks @costdev. Suggestion: Should we modify this ticket's title and description to include all escaping changes to this file? I guess that will be better for tracking purposes instead of splitting the task. What do you think?
Trac ticket: https://core.trac.wordpress.org/ticket/57336