Make WordPress Core

#56696 closed defect (bug) (fixed)

Multiple Themes: Correctly escape stylesheet URL

Reported by: alberuni-azad's profile Alberuni Azad. Owned by: desrosj's profile desrosj
Milestone: 6.1 Priority: normal
Severity: normal Version: 3.2
Component: Bundled Theme Keywords: has-patch commit
Focuses: Cc:

Description (last modified by audrasjb)

IN 'wp-content/themes/twentyeleven/header.php' on line '52' I've found that "bloginfo( 'stylesheet_url' )" was used without escaping. I know it's a silly issue. But I think we can improve it by escaping the URL for more consistency.

This has been discussed with a security team member who verified it's suitable as a public hardening issue.

Attachments (6)

56696.diff (847 bytes) - added by Alberuni Azad. 18 months ago.
Created patch.
56696.2.diff (1000 bytes) - added by Alberuni Azad. 18 months ago.
Created another path for escaping the get_template_directory_uri() function
56696.3.diff (1009 bytes) - added by Alberuni Azad. 18 months ago.
Uploaded another patch.
56696.4.diff (997 bytes) - added by Alberuni Azad. 18 months ago.
Created another patch for Twenty Eleven
56696.5.diff (2.1 KB) - added by Alberuni Azad. 18 months ago.
Created a separated patch for Twenty twelve, thirteen and fourteen
56696.6.diff (10.5 KB) - added by desrosj 18 months ago.

Download all attachments as: .zip

Change History (18)

@Alberuni Azad.
18 months ago

Created patch.

@Alberuni Azad.
18 months ago

Created another path for escaping the get_template_directory_uri() function

#1 @Alberuni Azad.
18 months ago

  • Keywords has-patch added

#2 @audrasjb
18 months ago

  • Description modified (diff)
  • Summary changed from This has been discussed with a security team member who verified it's suitable as a public hardening issue. to Twenty Eleven: Correctly escape stylesheet URL

#3 @costdev
18 months ago

  • Keywords changes-requested added

Hi @alberuni-azad, thanks for the patch! Can you update the patch so that this

esc_url( bloginfo( 'stylesheet_url' ) );

is changed to:

echo esc_url( get_bloginfo( 'stylesheet_url' ) );

Thanks!

Last edited 18 months ago by costdev (previous) (diff)

@Alberuni Azad.
18 months ago

Uploaded another patch.

#4 @Alberuni Azad.
18 months ago

Hi @costdev

Uploaded another patch https://core.trac.wordpress.org/attachment/ticket/56696/56696.3.diff

Please check now.

Thanks!

#5 @sabernhardt
18 months ago

  1. Could we use the get_stylesheet_uri() function instead of get_bloginfo?
  2. The stylesheet version date will be updated on #56450 (already mentioned in PR 3148 review).
  3. The same escaping could be added in Twenty Ten's header.php.
  4. The template directory function is also unescaped for Twenty Twelve to Twenty Fourteen, but we may remove those HTML5 scripts soon anyway (#56699). Twenty Fifteen already escapes the function.

#6 @Alberuni Azad.
18 months ago

Hi @sabernhardt

  1. Since get_bloginfo( 'stylesheet_url' ) function using the get_stylesheet_uri() we can directly call that function instead of the bloginfo function.
  2. I'm unsure which version this patch will be added, so I think once the ticket is confirmed, we can update the version date.

3 & 4. Yes, I've checked those files and noticed that the function is unescaped. Should I upload another patch for those themes too?

Anyway, Thank you so much for reviewing this ticket.

#7 @sabernhardt
18 months ago

If you would like to create patches for the other themes, too, that would be good :)

Then a committer can decide to change any or all of the themes.

Perhaps you could leave the stylesheet's modified date for now, to be edited on #56450 (or a later ticket) within days of the final release.

Last edited 18 months ago by sabernhardt (previous) (diff)

@Alberuni Azad.
18 months ago

Created another patch for Twenty Eleven

@Alberuni Azad.
18 months ago

Created a separated patch for Twenty twelve, thirteen and fourteen

#8 @Alberuni Azad.
18 months ago

@sabernhardt

I've uploaded 2 different patches. No 4 for the Twenty eleven and 5 no for the Twenty twelve, thirteen, and fourteen themes.

#9 @desrosj
18 months ago

  • Keywords changes-requested removed
  • Milestone changed from Awaiting Review to 6.1
  • Owner set to desrosj
  • Status changed from new to reviewing

#10 @desrosj
18 months ago

In 54404:

Twenty Eleven: Pass template directory URLs through esc_url().

Props alberuni-azad, sabernhardt, costdev.
Fixes #56717. See #56696.

@desrosj
18 months ago

#11 @desrosj
18 months ago

  • Keywords commit added
  • Summary changed from Twenty Eleven: Correctly escape stylesheet URL to Multiple Themes: Correctly escape stylesheet URL

56696.6.diff has a few more instances of this issue.

#12 @desrosj
18 months ago

  • Resolution set to fixed
  • Status changed from reviewing to closed

In 54405:

Bundled Themes: Properly escape URLs.

This adds output escaping to several theme related URLs.

Props alberuni-azad, sabernhardt.
Fixes #56696.

Note: See TracTickets for help on using tickets.