Make WordPress Core

Opened 2 years ago

Closed 2 years ago

#56145 closed defect (bug) (fixed)

unescaped 'home_url()' in 'wp-admin/themes.php' file in 'line 269'

Reported by: obayedmamur's profile obayedmamur Owned by: desrosj's profile desrosj
Milestone: 6.1 Priority: normal
Severity: normal Version:
Component: Themes Keywords: has-patch commit
Focuses: administration Cc:

Description

Hi there, 🙂

It's my first ticket at WordPress core.

I've found that in 'wp-admin/themes.php' file, in line number 269 there's 'home_url()' used without escaping. I think it should be escaped.

Attachments (2)

56145.patch (771 bytes) - added by hurayraiit 2 years ago.
Good finding! This should do the trick. :-)
56145_1.patch (1.2 KB) - added by hurayraiit 2 years ago.

Download all attachments as: .zip

Change History (17)

#1 @hztyfoon
2 years ago

Hey @obayedmamur, thanks for your contribution to WordPress. Hope you'll continue your contribution.

@hurayraiit
2 years ago

Good finding! This should do the trick. :-)

#2 @hztyfoon
2 years ago

  • Keywords has-patch added

#3 @costdev
2 years ago

  • Keywords commit added
  • Milestone changed from Awaiting Review to 6.1
  • Version trunk deleted

Hi @obayedmamur, welcome to Trac and thanks for the patch!

The patch looks good to me. 👍

Version 1, edited 2 years ago by costdev (previous) (next) (diff)

#4 @hurayraiit
2 years ago

Hi @costdev,
Thank you so much.

Last edited 2 years ago by hurayraiit (previous) (diff)

#5 follow-up: @desrosj
2 years ago

  • Keywords changes-requested added; commit removed

@hurayraiit It looks like there is a second instance of home_url() not being wrapped with esc_url() that 56145.patch fixes, but the one mentioned in the original ticket title is not corrected.

Could you change both at the same time in one patch?

#6 @desrosj
2 years ago

  • Component changed from Administration to Themes
  • Focuses coding-standards removed

#7 in reply to: ↑ 5 @hurayraiit
2 years ago

Replying to desrosj:

@hurayraiit It looks like there is a second instance of home_url() not being wrapped with esc_url() that 56145.patch fixes, but the one mentioned in the original ticket title is not corrected.

Could you change both at the same time in one patch?

Yeah, absolutely. Let me check please.

@hurayraiit
2 years ago

#8 @hurayraiit
2 years ago

Hi @desrosj
Thanks for the comment. I have attached the patch for line 273 also in the second attachment(56145_1.patch). Please let me know if there's anything else I can do.

#9 @costdev
2 years ago

Related ticket: #56146

#10 @obayedmamur
2 years ago

Thanks to everyone for your quick response! Loved it! 💖

#11 @hztyfoon
2 years ago

Related ticket: #56132, #56133, #56146

#12 @hztyfoon
2 years ago

Both patches looks good to me. Good Work. Thanks @hurayraiit for your contribution.

#13 @desrosj
2 years ago

#56146 was marked as a duplicate.

#14 @desrosj
2 years ago

  • Keywords commit added; changes-requested removed

#15 @desrosj
2 years ago

  • Owner set to desrosj
  • Resolution set to fixed
  • Status changed from new to closed

In 53677:

Themes: Properly escape home_url() when changing and updating themes.

Props obayedmamur, hurayraiit, costdev, shraboni, msnewas.
Fixes #56145.

Note: See TracTickets for help on using tickets.