diff --git a/CHANGELOG.md b/CHANGELOG.md index d7d2cc94..803ad985 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,16 @@ * New `Backups` configuration panel in tools * New `Cache::purge()` option in cache drop-down to clear out old cache only +# v1.8.11 +## mm/dd/2018 + +1. [](#improved) + * Change usage of basename where possible [#1480](https://github.com/getgrav/grav-plugin-admin/pull/1480) + * Improved filename validation (requires Grav 1.5.3) +1. [](#bugfix) + * File Uploads: Do not trust mimetype sent by the browser + * Fixed file extension detection + # v1.8.10 ## 10/01/2018 diff --git a/classes/admin.php b/classes/admin.php index f1b2ecbd..4c9f9054 100644 --- a/classes/admin.php +++ b/classes/admin.php @@ -170,11 +170,11 @@ class Admin /** @var \DirectoryIterator $directory */ foreach (new \DirectoryIterator($path) as $file) { - if ($file->isDir() || $file->isDot() || Utils::startsWith($file->getBasename(), '.')) { + if ($file->isDir() || $file->isDot() || Utils::startsWith($file->getFilename(), '.')) { continue; } - $lang = basename($file->getBasename(), '.yaml'); + $lang = $file->getBasename('.yaml'); $languages[$lang] = LanguageCodes::getNativeName($lang); @@ -202,7 +202,7 @@ class Admin if ($file->isDir() || !preg_match('/^[^.].*.yaml$/', $file->getFilename())) { continue; } - $configurations[] = basename($file->getBasename(), '.yaml'); + $configurations[] = $file->getBasename('.yaml'); } return $configurations; @@ -674,10 +674,10 @@ class Admin $obj->file = $file; $obj->page = $this->grav['pages']->get(dirname($obj->path)); - $filename = pathinfo($obj->title)['filename']; - $filename = str_replace(['@3x', '@2x'], '', $filename); - if (isset(pathinfo($obj->title)['extension'])) { - $filename .= '.' . pathinfo($obj->title)['extension']; + $fileInfo = pathinfo($obj->title); + $filename = str_replace(['@3x', '@2x'], '', $fileInfo['filename']); + if (isset($fileInfo['extension'])) { + $filename .= '.' . $fileInfo['extension']; } if ($obj->page && isset($obj->page->media()[$filename])) { diff --git a/classes/adminbasecontroller.php b/classes/adminbasecontroller.php index a474076e..154aab6a 100644 --- a/classes/adminbasecontroller.php +++ b/classes/adminbasecontroller.php @@ -227,10 +227,10 @@ class AdminBaseController $upload = $this->normalizeFiles($_FILES['data'], $settings->name); - $filename = trim($upload->file->name); + $filename = $upload->file->name; // Handle bad filenames. - if (strtr($filename, "\t\n\r\0\x0b", '_____') !== $filename || rtrim($filename, '. ') !== $filename || preg_match('|\.php|', $filename)) { + if (!Utils::checkFilename($filename)) { $this->admin->json_response = [ 'status' => 'error', 'message' => sprintf($this->admin->translate('PLUGIN_ADMIN.FILEUPLOAD_UNABLE_TO_UPLOAD', null), @@ -287,6 +287,9 @@ class AdminBaseController $accepted = false; $errors = []; + // Do not trust mimetype sent by the browser + $mime = Utils::getMimeByFilename($upload->file->name); + foreach ((array)$settings->accept as $type) { // Force acceptance of any file when star notation if ($type === '*') { @@ -295,15 +298,24 @@ class AdminBaseController } $isMime = strstr($type, '/'); - $find = str_replace('*', '.*', $type); + $find = str_replace(['.', '*'], ['\.', '.*'], $type); - $match = preg_match('#' . $find . '$#', $isMime ? $upload->file->type : $upload->file->name); - if (!$match) { - $message = $isMime ? 'The MIME type "' . $upload->file->type . '"' : 'The File Extension'; - $errors[] = $message . ' for the file "' . $upload->file->name . '" is not an accepted.'; - $accepted |= false; + if ($isMime) { + $match = preg_match('#' . $find . '$#', $mime); + if (!$match) { + $errors[] = 'The MIME type "' . $mime . '" for the file "' . $upload->file->name . '" is not an accepted.'; + } else { + $accepted = true; + break; + } } else { - $accepted |= true; + $match = preg_match('#' . $find . '$#', $upload->file->name); + if (!$match) { + $errors[] = 'The File Extension for the file "' . $upload->file->name . '" is not an accepted.'; + } else { + $accepted = true; + break; + } } } @@ -368,7 +380,7 @@ class AdminBaseController // Generate random name if required if ($settings->random_name) { // TODO: document - $extension = pathinfo($upload->file->name)['extension']; + $extension = pathinfo($upload->file->name, PATHINFO_EXTENSION); $upload->file->name = Utils::generateRandomString(15) . '.' . $extension; } diff --git a/classes/admincontroller.php b/classes/admincontroller.php index b74b484b..665f1756 100644 --- a/classes/admincontroller.php +++ b/classes/admincontroller.php @@ -1758,6 +1758,19 @@ class AdminController extends AdminBaseController return false; } + $filename = $_FILES['file']['name']; + + // Handle bad filenames. + if (!Utils::checkFilename($filename)) { + $this->admin->json_response = [ + 'status' => 'error', + 'message' => sprintf($this->admin->translate('PLUGIN_ADMIN.FILEUPLOAD_UNABLE_TO_UPLOAD'), + $filename, 'Bad filename') + ]; + + return false; + } + $grav_limit = $config->get('system.media.upload_limit', 0); // You should also check filesize here. if ($grav_limit > 0 && $_FILES['file']['size'] > $grav_limit) { @@ -1771,18 +1784,13 @@ class AdminController extends AdminBaseController // Check extension - $fileParts = pathinfo($_FILES['file']['name']); - - $fileExt = ''; - if (isset($fileParts['extension'])) { - $fileExt = strtolower($fileParts['extension']); - } + $extension = strtolower(pathinfo($filename, PATHINFO_EXTENSION)); // If not a supported type, return - if (!$fileExt || !$config->get("media.types.{$fileExt}")) { + if (!$extension || !$config->get("media.types.{$extension}")) { $this->admin->json_response = [ 'status' => 'error', - 'message' => $this->admin->translate('PLUGIN_ADMIN.UNSUPPORTED_FILE_TYPE') . ': ' . $fileExt + 'message' => $this->admin->translate('PLUGIN_ADMIN.UNSUPPORTED_FILE_TYPE') . ': ' . $extension ]; return false; @@ -1808,7 +1816,7 @@ class AdminController extends AdminBaseController // Upload it if (!move_uploaded_file($_FILES['file']['tmp_name'], - sprintf('%s/%s', $path, $_FILES['file']['name'])) + sprintf('%s/%s', $path, $filename)) ) { $this->admin->json_response = [ 'status' => 'error', @@ -1820,13 +1828,12 @@ class AdminController extends AdminBaseController // Add metadata if needed $include_metadata = Grav::instance()['config']->get('system.media.auto_metadata_exif', false); - $filename = $fileParts['basename']; - $filename = str_replace(['@3x', '@2x'], '', $filename); + $basename = str_replace(['@3x', '@2x'], '', pathinfo($filename, PATHINFO_BASENAME)); $metadata = []; - if ($include_metadata && isset($media[$filename])) { - $img_metadata = $media[$filename]->metadata(); + if ($include_metadata && isset($media[$basename])) { + $img_metadata = $media[$basename]->metadata(); if ($img_metadata) { $metadata = $img_metadata; } @@ -1869,6 +1876,11 @@ class AdminController extends AdminBaseController $filename = !empty($this->post['filename']) ? $this->post['filename'] : null; + // Handle bad filenames. + if (!Utils::checkFilename($filename)) { + $filename = null; + } + if (!$filename) { $this->admin->json_response = [ 'status' => 'error', @@ -1957,7 +1969,7 @@ class AdminController extends AdminBaseController protected function taskProcessMarkdown() { if (!$this->authorizeTask('process markdown', ['admin.pages', 'admin.super'])) { - return; + return false; } try { @@ -2347,6 +2359,16 @@ class AdminController extends AdminBaseController } $file_path = $_FILES['uploaded_file']['tmp_name']; + + // Handle bad filenames. + if (!Utils::checkFilename($file_path)) { + $this->admin->json_response = [ + 'status' => 'error', + 'message' => $this->admin->translate('PLUGIN_ADMIN.UNKNOWN_ERRORS') + ]; + + return false; + } }