Make WordPress Core

Opened 6 years ago

Closed 6 years ago

#12280 closed defect (bug) (invalid)

^ is an escape char in the windows OS

Reported by: hakre Owned by: ryan
Milestone: Priority: normal
Severity: normal Version: 3.0
Component: Security Keywords: has-patch
Focuses: Cc:


It should be removed from filenames then, otherwise the sanitize_file_name function could be easily tricked on windows servers.

Related: #9416, [11178]

Attachments (1)

12280.patch (863 bytes) - added by hakre 6 years ago.

Download all attachments as: .zip

Change History (8)

6 years ago

#1 @miqrogroove
6 years ago

What is the precedent? Windows always allows karats in filenames doesn't it?

#2 @hakre
6 years ago

is an escape char in the windows OS. so it has a special meaning. those are to be removed from filenames as per definition of function sanitize_file_name():

 * Removes special characters that are illegal in filenames on certain
 * operating systems and special characters requiring special escaping
 * to manipulate at the command line. [...]

See section "Escape Character" in http://ss64.com/nt/syntax-esc.html

#3 @miqrogroove
6 years ago

-1 invalid. Filename escaping is the responsibility of the API, which is PHP in this case. Tested and confirmed:

copy('index.php', 'inde^^x.php');

PHP creates a file named inde^^x.php, as expected. If there were a legitimate problem here, it would have created a file named inde^x.php.

#4 @hakre
6 years ago

that argumentation would make -1 for those (which are in right now) as well:

"?", "[", "]", "/", "\\", "=", "<", ">", ":", ";", ",", "'", "\"", "&", "$", "#", "*", "(", ")", "|", "~", "`", "!", "{", "}"

because the API, which is PHP in this case, would be responsible for filename escaping.

So where does that lead to?

#5 @miqrogroove
6 years ago

You are welcome to test all of those characters. Half of them are valid in Windows and will cause no problem. The other half are invalid and will throw a PHP Warning, which is not desirable. All other characters are valid and should not need escaping above the PHP layer.

#6 @hakre
6 years ago

I'm not in here to remove any of those. I created that ticket to add another special char that might be wise to replace for shell usage on the WINNT platform. Just as a precaution while I was stumbling over that line of code.

Infact, I do not care much about the windows platform and WP. So from that point of view, not worth the discussion for me now. Commit it or close it I won't care any longer. Enough typed here.

#7 @nacin
6 years ago

  • Milestone 3.0 deleted
  • Resolution set to invalid
  • Status changed from new to closed
Note: See TracTickets for help on using tickets.