Bug #21165
closedFilenames should be escaped with escapeshellarg before passing them to imagemagick
0%
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
Updated by Oliver Klee about 15 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.
Updated by Helmut Hummel about 15 years ago
+1 by reading (no need to test, easy enough)
Updated by Ernesto Baschny about 15 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??
Updated by Rupert Germann about 15 years ago
but will fixPermissions() (or better chmod() and chgrp()) work on files with unexcaped spaces?
Updated by Ernesto Baschny about 15 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
Updated by Ernesto Baschny about 15 years ago
See trunk-v2 for a proposed solution.
Updated by Oliver Klee about 15 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.
Updated by Oliver Klee about 15 years ago
comments.diff improves the comments on t3lib_div::fixPermissions (no code changes, just comments).
Updated by Oliver Hader about 15 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().
Updated by Ernesto Baschny about 15 years ago
Commited to:
trunk (rev.6258 = beta2)
TYPO3_4-2 (rev.6259 = 4.2.10)
TYPO3_4-1 (rev.6260 = 4.1.13)
Updated by Helmut Hummel about 15 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
Updated by Ernesto Baschny about 15 years ago
Commited follow-up by Oli Hader (thanks) to:
trunk (rev.6262)
TYPO3_4-2 (rev.6263)
TYPO3_4-1 (rev.6264)