WordPress.org

Make WordPress Core

Opened 4 years ago

Last modified 4 years ago

#14148 new defect (bug)

wp_get_attachment_url() is not url encoding

Reported by: danorton Owned by:
Milestone: Future Release Priority: normal
Severity: major Version: 3.0
Component: Security Keywords: has-patch needs-testing
Focuses: Cc:

Description

A fairly fundamental flaw, the function wp_get_attachment_url() doesn't return a valid URL if the filename contains unescaped URL characters.

I'm not sure, but this might be a security issue, as the current version can generate URLs that don't match the filename, but instead passes query parameters back to the server.

The attached patch for Version 3.0 file fixes this in wp-includes/post.php

Attachments (2)

post.php.patch (1.6 KB) - added by danorton 4 years ago.
14148.diff (2.8 KB) - added by kawauso 4 years ago.
Compact patch based on danorton's but adding corrections for thumbnails and intermediate sizes. Tidy up comments a bit too.

Download all attachments as: .zip

Change History (7)

danorton4 years ago

comment:1 follow-up: nacin4 years ago

Can you post an example URL and what would get returned with and without the patch?

comment:2 in reply to: ↑ 1 danorton4 years ago

Replying to nacin:

Can you post an example URL and what would get returned with and without the patch?

For a file named "X%X.txt"

Currently the URL returned is:
.../wp-content/uploads/2010/06/X%X.txt

This patch corrects it to return:
.../wp-content/uploads/2010/06/X%25X.txt

comment:3 nacin4 years ago

  • Component changed from General to Security
  • Milestone changed from Awaiting Review to 3.1

kawauso4 years ago

Compact patch based on danorton's but adding corrections for thumbnails and intermediate sizes. Tidy up comments a bit too.

comment:4 kawauso4 years ago

  • Cc otterish@… added
  • Keywords has-patch needs-testing added; url query removed

Had to force image_downsize() to use encoded filenames when replacing which may break things. Any other ideas?

comment:5 ryan4 years ago

  • Milestone changed from 3.1 to Future Release

This is too risky for 3.1 and will require thorough testing.

Note: See TracTickets for help on using tickets.