Fixes for custom theme upload:

* skip more files (.xcf image sources, .html docs)
* skip files before rejecting them for funky filenames!
* allow period in filenames (eg foo-1.4.ttf) but blacklist some unsafe extensions-within-extensions
This commit is contained in:
Brion Vibber 2010-09-02 12:11:45 -07:00
parent 4cbbfdab84
commit 11f7fce3bb

View File

@ -128,8 +128,16 @@ class ThemeUploader
continue; continue;
} }
// Check the directory structure... // Is this a safe or skippable file?
$path = pathinfo($name); $path = pathinfo($name);
if ($this->skippable($path['filename'], $path['extension'])) {
// Documentation and such... booooring
continue;
} else {
$this->validateFile($path['filename'], $path['extension']);
}
// Check the directory structure...
$dirs = explode('/', $path['dirname']); $dirs = explode('/', $path['dirname']);
$baseDir = array_shift($dirs); $baseDir = array_shift($dirs);
if ($commonBaseDir === false) { if ($commonBaseDir === false) {
@ -144,14 +152,6 @@ class ThemeUploader
$this->validateFileOrFolder($dir); $this->validateFileOrFolder($dir);
} }
// Is this a safe or skippable file?
if ($this->skippable($path['filename'], $path['extension'])) {
// Documentation and such... booooring
continue;
} else {
$this->validateFile($path['filename'], $path['extension']);
}
$fullPath = $dirs; $fullPath = $dirs;
$fullPath[] = $path['basename']; $fullPath[] = $path['basename'];
$localFile = implode('/', $fullPath); $localFile = implode('/', $fullPath);
@ -180,9 +180,12 @@ class ThemeUploader
} }
} }
/**
* @fixme Probably most unrecognized files should just be skipped...
*/
protected function skippable($filename, $ext) protected function skippable($filename, $ext)
{ {
$skip = array('txt', 'rtf', 'doc', 'docx', 'odt'); $skip = array('txt', 'html', 'rtf', 'doc', 'docx', 'odt', 'xcf');
if (strtolower($filename) == 'readme') { if (strtolower($filename) == 'readme') {
return true; return true;
} }
@ -201,17 +204,24 @@ class ThemeUploader
protected function validateFileOrFolder($name) protected function validateFileOrFolder($name)
{ {
if (!preg_match('/^[a-z0-9_-]+$/i', $name)) { if (!preg_match('/^[a-z0-9_\.-]+$/i', $name)) {
$msg = _("Theme contains invalid file or folder name. " . $msg = _("Theme contains invalid file or folder name. " .
"Stick with ASCII letters, digits, underscore, and minus sign."); "Stick with ASCII letters, digits, underscore, and minus sign.");
throw new ClientException($msg); throw new ClientException($msg);
} }
if (preg_match('/\.(php|cgi|asp|aspx|js|vb)\w/i', $name)) {
$msg = _("Theme contains unsafe file extension names; may be unsafe.");
throw new ClientException($msg);
}
return true; return true;
} }
protected function validateExtension($ext) protected function validateExtension($ext)
{ {
$allowed = array('css', 'png', 'gif', 'jpg', 'jpeg'); $allowed = array('css', // CSS may need validation
'png', 'gif', 'jpg', 'jpeg',
'svg', // SVG images/fonts may need validation
'ttf', 'eot', 'woff');
if (!in_array(strtolower($ext), $allowed)) { if (!in_array(strtolower($ext), $allowed)) {
$msg = sprintf(_("Theme contains file of type '.%s', " . $msg = sprintf(_("Theme contains file of type '.%s', " .
"which is not allowed."), "which is not allowed."),