Make WordPress Core

Opened 3 years ago

Closed 3 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 3 years ago.
Good finding! This should do the trick. :-)
56145_1.patchโ€‹ (1.2 KB) - added by hurayraiit 3 years ago.

Download all attachments as: .zip

Change History (17)

#1 @hztyfoon
3 years ago

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

@hurayraiit
3 years ago

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

#2 @hztyfoon
3 years ago

  • Keywords has-patch added

#3 @costdev
3 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 3 years ago by costdev (previous) (next) (diff)

#4 @hurayraiit
3 years ago

Hi @costdev,
Thank you so much.

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

#5 follow-up: @desrosj
3 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
3 years ago

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

#7 in reply to: โ†‘ย 5 @hurayraiit
3 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.

#8 @hurayraiit
3 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
3 years ago

Related ticket: #56146

#10 @obayedmamur
3 years ago

Thanks to everyone for your quick response! Loved it! ๐Ÿ’–

#11 @hztyfoon
3 years ago

Related ticket: #56132, #56133, #56146

#12 @hztyfoon
3 years ago

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

#13 @desrosj
3 years ago

#56146 was marked as a duplicate.

#14 @desrosj
3 years ago

  • Keywords commit added; changes-requested removed

#15 @desrosj
3 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.