diff --git a/CHANGELOG.md b/CHANGELOG.md index 1b401d3c..2a90f00b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,7 @@ * Catch ValidationException to avoid potential fatal error * Fixed regression issue on reset fields * Removed `required` attribute in individual checkboxes as it forces all to be checked + * Security fix to ensure file uploads are not manipulated mid-post - thnx @FLH! # v2.10.0 ## 10/26/2017 diff --git a/classes/form.php b/classes/form.php index c3e51a86..b8d5ee38 100644 --- a/classes/form.php +++ b/classes/form.php @@ -395,37 +395,21 @@ public function uploadFiles() ]; } - // Remove the error object to avoid storing it - unset($upload->file->error); - - // we need to move the file at this stage or else - // it won't be available upon save later on - // since php removes it from the upload location - $tmp_dir = $grav['locator']->findResource('tmp://', true, true); - $tmp_file = $upload->file->tmp_name; - $tmp = $tmp_dir . '/uploaded-files/' . basename($tmp_file); - - Folder::create(dirname($tmp)); - if (!move_uploaded_file($tmp_file, $tmp)) { - // json_response - return [ - 'status' => 'error', - 'message' => sprintf($grav['language']->translate('PLUGIN_FORM.FILEUPLOAD_UNABLE_TO_MOVE', null, true), '', $tmp) - ]; - } - - $upload->file->tmp_name = $tmp; - - // Handle file size limits - $settings->filesize *= self::BYTES_TO_MB; // 1024 * 1024 [MB in Bytes] - if ($settings->filesize > 0 && $upload->file->size > $settings->filesize) { - // json_response - return [ + // Handle bad filenames. + $filename = $upload->file->name; + if (strtr($filename, "\t\n\r\0\x0b", '_____') !== $filename || rtrim($filename, ". ") !== $filename || preg_match('|\.php|', $filename)) { + $this->admin->json_response = [ 'status' => 'error', - 'message' => $grav['language']->translate('PLUGIN_FORM.EXCEEDED_GRAV_FILESIZE_LIMIT') + 'message' => sprintf($this->admin->translate('PLUGIN_ADMIN.FILEUPLOAD_UNABLE_TO_UPLOAD', null), + $filename, 'Bad filename') ]; + + return false; } + // Remove the error object to avoid storing it + unset($upload->file->error); + // Handle Accepted file types // Accept can only be mime types (image/png | image/*) or file extensions (.pdf|.jpg) @@ -459,6 +443,36 @@ public function uploadFiles() ]; } + + // Handle file size limits + $settings->filesize *= self::BYTES_TO_MB; // 1024 * 1024 [MB in Bytes] + if ($settings->filesize > 0 && $upload->file->size > $settings->filesize) { + // json_response + return [ + 'status' => 'error', + 'message' => $grav['language']->translate('PLUGIN_FORM.EXCEEDED_GRAV_FILESIZE_LIMIT') + ]; + } + + + // we need to move the file at this stage or else + // it won't be available upon save later on + // since php removes it from the upload location + $tmp_dir = $grav['locator']->findResource('tmp://', true, true); + $tmp_file = $upload->file->tmp_name; + $tmp = $tmp_dir . '/uploaded-files/' . basename($tmp_file); + + Folder::create(dirname($tmp)); + if (!move_uploaded_file($tmp_file, $tmp)) { + // json_response + return [ + 'status' => 'error', + 'message' => sprintf($grav['language']->translate('PLUGIN_FORM.FILEUPLOAD_UNABLE_TO_MOVE', null, true), '', $tmp) + ]; + } + + $upload->file->tmp_name = $tmp; + // Retrieve the current session of the uploaded files for the field // and initialize it if it doesn't exist $sessionField = base64_encode($uri);