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)

@hakre6 years ago

comment:1 @miqrogroove6 years ago

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

comment:2 @hakre6 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

comment:3 @miqrogroove6 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.

comment:4 @hakre6 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?

comment:5 @miqrogroove6 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.

comment:6 @hakre6 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.

comment:7 @nacin6 years ago

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