Project

General

Profile

Actions

Bug #21165

closed

Filenames should be escaped with escapeshellarg before passing them to imagemagick

Added by Helmut Hummel over 14 years ago. Updated almost 14 years ago.

Status:
Closed
Priority:
Should have
Category:
-
Target version:
-
Start date:
2009-09-30
Due date:
% Done:

0%

Estimated time:
TYPO3 Version:
4.2
PHP Version:
4.3
Tags:
Complexity:
Is Regression:
Sprint Focus:

Description

The problem lies in class.t3lib_stdgraphic.php::wrapFileName() with is
supposed to wrap a filename into double quotes.
Unfortunately it does this task only, if it finds a space character
in the filename. Other special characters which are not allowed on
the shell are not treated.

The better option would be to use escapeshellarg() provided by PHP.

Reported by: Christian Welzel
OTRS ID: 2009080710000036
(issue imported from #M12090)


Files

12090_41.diff (891 Bytes) 12090_41.diff Administrator Admin, 2009-10-07 19:59
12090_trunk.diff (3.7 KB) 12090_trunk.diff Administrator Admin, 2009-10-07 20:03
12090_42.diff (1.02 KB) 12090_42.diff Administrator Admin, 2009-10-07 20:03
12090_trunk-v2.diff (1.46 KB) 12090_trunk-v2.diff Administrator Admin, 2009-10-22 11:32
comments.diff (571 Bytes) comments.diff Administrator Admin, 2009-10-22 11:48
12090_followup_trunk_and_42.diff (824 Bytes) 12090_followup_trunk_and_42.diff Administrator Admin, 2009-10-22 13:30
12090_followup_41.diff (842 Bytes) 12090_followup_41.diff Administrator Admin, 2009-10-22 13:30

Related issues 3 (0 open3 closed)

Related to TYPO3 Core - Bug #21360: Image Generation broken with PHP safe_mode = On / GraphicsmagickClosedBenni Mack2009-10-26

Actions
Related to TYPO3 Core - Bug #21609: Problem with german "Umlaute" in BE file list, no thumbs generatedClosedJigal van Hemert2009-11-18

Actions
Related to TYPO3 Core - Bug #21087: If safe_mode is enabled, thumb generation fails for file with "&" in filenameClosed2009-09-18

Actions
Actions #1

Updated by Oliver Klee over 14 years ago

The 4.2 and 4.1 patches are different because 4.1 doesn't require PHP5 and thus cannot use the "protected" keyword. In addition, my editor removed a trailing space that was there only in the 4.2 branch, but not in the trunk or the 4.1 branch.

Actions #2

Updated by Helmut Hummel over 14 years ago

+1 by reading (no need to test, easy enough)

Actions #3

Updated by Ernesto Baschny over 14 years ago

The function wrapFileName() is also used in these situations:

t3lib_div::fixPermissions($this->wrapFileName($output));

So calling that on $output with a file with a single quote inside will probably make fixPermission unable to work on that file.

I guess we don't need wrapFileName() when calling fixPermissions, because fixPermissions doesn't call a shell but uses internal PHP functions to work on the file. So we need to remove the wrapFileName within the fixPermissions calls. What do you think??

Actions #4

Updated by Rupert Germann over 14 years ago

but will fixPermissions() (or better chmod() and chgrp()) work on files with unexcaped spaces?

Actions #5

Updated by Ernesto Baschny over 14 years ago

Yes, it works on the filename as it is (on the file system). If we escape it, it won't work anymore:

$file = 'this \' is a test.txt';
touch($file); # creates file: this ' is a test.txt

chgrp($file, 'crondevel'); # works

$file = escapeshellarg($file);
chgrp($file, 'crondevel'); # chokes

Actions #6

Updated by Ernesto Baschny over 14 years ago

See trunk-v2 for a proposed solution.

Actions #7

Updated by Oliver Klee over 14 years ago

+1 on reading. I'll create an additional patch in a minute that improves the documentation for the fixPermissions function so we won't need to puzzle about this in the future.

Actions #8

Updated by Oliver Klee over 14 years ago

comments.diff improves the comments on t3lib_div::fixPermissions (no code changes, just comments).

Actions #9

Updated by Oliver Hader over 14 years ago

+1 by reading and testing in 4.3-Trunk

Besides that I tested the following:
touch('File with spaces.txt'); -> works
touch(escapeshellarg('File with spaces.txt')); -> did not work
Thus, the wrapFileName() call can be removed before passing the file to fixPermissions().

Actions #10

Updated by Ernesto Baschny over 14 years ago

Commited to:
trunk (rev.6258 = beta2)
TYPO3_4-2 (rev.6259 = 4.2.10)
TYPO3_4-1 (rev.6260 = 4.1.13)

Actions #11

Updated by Helmut Hummel over 14 years ago

in thumbs.php there is also a wrapFileName function, which does exactly the same the old one did. It hast to be replaced with a call to t3lib_stdgraphic or this function must be adapted to also escape the filenames

Actions #12

Updated by Oliver Klee over 14 years ago

I'll take care of the follow-up.

Actions #13

Updated by Oliver Hader over 14 years ago

+1 by reading the follow-ups

Actions #14

Updated by Helmut Hummel over 14 years ago

+1 by reading the follow-ups

Actions #15

Updated by Ernesto Baschny over 14 years ago

Commited follow-up by Oli Hader (thanks) to:
trunk (rev.6262)
TYPO3_4-2 (rev.6263)
TYPO3_4-1 (rev.6264)

Actions #16

Updated by Ernesto Baschny over 14 years ago

reopened to make it public

Actions

Also available in: Atom PDF