WordPress.org

Make WordPress Core

Opened 22 months ago

Closed 9 months ago

#21241 closed defect (bug) (fixed)

Default value for background_image_thumb fails when background_image URL includes a percent sign

Reported by: cfinke Owned by: nacin
Milestone: 3.6 Priority: normal
Severity: minor Version: 3.4.1
Component: Appearance Keywords: has-patch
Focuses: Cc:

Description

If the background_image theme mod is set to a URL that includes a percent sign, and if background_image_thumb is not set, the code that uses background_image as the default setting for background_image_thumb fails due to get_theme_mod interpreting the percent signs in the background_image URL as sprintf conversion specifications.

I don't think that this situation can occur in a vanilla WordPress install (but that might be host-specific), but it's entirely possible that a plugin would set background_image to a URL including a percent sign.

Attachments (1)

21241.patch (1.8 KB) - added by cfinke 22 months ago.
Replaces single percent signs with double percent signs in anticipation of the sprintf call in get_theme_mod.

Download all attachments as: .zip

Change History (9)

cfinke22 months ago

Replaces single percent signs with double percent signs in anticipation of the sprintf call in get_theme_mod.

comment:1 SergeyBiryukov21 months ago

  • Milestone Awaiting Review deleted
  • Resolution set to duplicate
  • Status changed from new to closed

It's an issue with uploading any files containing percent signs in their names, not just with custom background. Going to close as a duplicate of #16330, since the patch there resolves this as well.

Last edited 21 months ago by SergeyBiryukov (previous) (diff)

comment:2 cfinke21 months ago

  • Resolution duplicate deleted
  • Status changed from closed to reopened

I disagree that this is a duplicate of #16330. That bug concerns sanitizing the filenames of uploaded media; this bug concerns passing a string through sprintf() without considering that it could include strings that look like conversion specifications but aren't.

None of the fixes being discussed in #16330 would fix the bug presented by a plugin setting background_image to a URL like http://example.com/some%2Fbackground%2Fimage%2Furl.png.

comment:3 SergeyBiryukov21 months ago

  • Milestone set to Awaiting Review

Indeed, I missed the sprintf() part. Thanks for the clarification.

comment:4 davouch15 months ago

  • Cc davouch added

comment:5 SergeyBiryukov15 months ago

  • Milestone changed from Awaiting Review to 3.6

comment:6 nacin10 months ago

This is goofy. At first I thought this should be added to get_theme_mod(), but then I realized that this is specifically the *default* value, which by design could take %s, so that's wrong. Then I thought that get_background_image() could return %s, but that's *also* wrong because it too goes through get_theme_mod(). Just wanted to describe my thought process before committing.

comment:7 ethitter9 months ago

  • Cc erick@… added

comment:8 nacin9 months ago

  • Owner set to nacin
  • Resolution set to fixed
  • Status changed from reopened to closed

In 24630:

When generating a background image thumbnail URL, escape percent signs for the eventual sprintf() call inside get_theme_mod().

props cfinke.
fixes #21241.

Note: See TracTickets for help on using tickets.