Make WordPress Core

Opened 5 years ago

Last modified 4 years ago

#47539 reviewing enhancement

Incomplete sanitization of upload file name.

Reported by: mt8biz's profile mt8.biz Owned by: sergeybiryukov's profile SergeyBiryukov
Milestone: Future Release Priority: normal
Severity: normal Version: 2.1.1
Component: Media Keywords: has-patch
Focuses: Cc:

Description

The newline code is replaced with "-" in sanitize_file_name, but other control characters have not been sanitized.

For example, ^ P ( \ x10 )

This allows uploading with the control characters included in the file name.

# ls -la
-rw-r--r--  1 root     root     19058 Jun 14 04:21 ???wapuu_escape-150x150.png
-rw-r--r--  1 root     root     41163 Jun 14 04:21 ???wapuu_escape-297x300.png
-rw-r--r--  1 root     root     31022 Jun 14 04:21 ???wapuu_escape.png

After applying the patch:

# ls -la
-rw-r--r-- 1 www-data www-data 19058 Jun 14 04:27 wapuu_escape-150x150.png
-rw-r--r-- 1 www-data www-data 41163 Jun 14 04:27 wapuu_escape-297x300.png
-rw-r--r-- 1 www-data www-data 31022 Jun 14 04:27 wapuu_escape.png

Attach a test file.

Attachments (2)

formatting.php.diff (582 bytes) - added by mt8.biz 5 years ago.
wapuu.zip (31.5 KB) - added by mt8.biz 5 years ago.

Download all attachments as: .zip

Change History (7)

@mt8.biz
5 years ago

#1 @SergeyBiryukov
5 years ago

  • Milestone changed from Awaiting Review to 5.3
  • Owner set to SergeyBiryukov
  • Status changed from new to reviewing
  • Type changed from feature request to enhancement

This ticket was mentioned in Slack in #core-media by desrosj. View the logs.


5 years ago

#3 @kirasong
4 years ago

@SergeyBiryukov Are you still looking to land this for 5.3?

If not, will have to move to Future Release or 5.4, depending on your preferences, before 5.3 beta.

#4 @kirasong
4 years ago

  • Milestone changed from 5.3 to Future Release

As 5.3 beta is about to ship, going to go ahead and move this to Future Release.

Of course, feel free to milestone differently as you'd like.

#5 @mt8.biz
4 years ago

@SergeyBiryukov 
What do you think of this?

Although control characters are rarely included in file names, it is best to remove them.

Thank you

Note: See TracTickets for help on using tickets.