Sanitize url and title missing

[rafiq91]

1. The login title in wp-login.php is not sanitized properly ​https://prnt.sc/MJLEeeUWf7BE
2. Network site URL is also not properly sanitized ​https://prnt.sc/6rFVD0ClxbO-

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

[rajinsharwar]

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?

[costdev]

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.

[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

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

[costdev]

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 adding multisite focus, as this file is specifically for Multisite.
• Removing privacy as the proposed change does not affect privacy.

[rajinsharwar]

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?

[costdev]

@rajinsharwar I agree that one ticket makes sense for these changes, and the ticket title/description can be updated. Separate commits can be made if that's preferred by the final committer.

[oglekler]

I cannot find any possible side effects from this, but I didn't find anything for wp-login.php, and screenshot are not available, so, it isn't clear what should be escaped. In this file, there are a lot of calls for URLs to look through 😅