diff --git a/app/code/Magento/Customer/Model/Metadata/Form/File.php b/app/code/Magento/Customer/Model/Metadata/Form/File.php index a2a57c142dd90..d2abf74eab4f3 100644 --- a/app/code/Magento/Customer/Model/Metadata/Form/File.php +++ b/app/code/Magento/Customer/Model/Metadata/Form/File.php @@ -194,13 +194,81 @@ public function extractValue(\Magento\Framework\App\RequestInterface $request) */ protected function _validateByRules($value) { - $label = $value['name']; - $rules = $this->getAttribute()->getValidationRules(); - $extension = $this->ioFile->getPathInfo($value['name'])['extension']; - $fileExtensions = ArrayObjectSearch::getArrayElementByName( - $rules, - 'file_extensions' - ); + $label = $value['name'] ?? $value['file'] ?? ''; + $rules = $this->getAttribute()->getValidationRules() ?? []; + + // Extract and validate file name + $fileNameResult = $this->extractAndValidateFileName($value, $label); + if (!isset($fileNameResult['name'])) { + return $fileNameResult; // Return error array + } + $value['name'] = $fileNameResult['name']; + $label = $fileNameResult['label']; + + // Validate file extension + $extensionResult = $this->validateFileExtension($value['name'], $rules, $label); + if (!empty($extensionResult)) { + return $extensionResult; + } + + // Validate file path + $filePath = $this->getFilePath($value, $label); + if (is_array($filePath)) { + return $filePath; // Return error array + } + + // Validate file size + return $this->validateFileSize($value, $rules, $label); + } + + /** + * Extract and validate file name from value + * + * @param array $value + * @param string $label + * @return array Returns array with name and label or error array + */ + private function extractAndValidateFileName(array $value, string $label): array + { + // For UI component uploads, get name from file path if not provided + if (empty($value['name']) && !empty($value['file'])) { + // Validate file path for security before extracting filename + if (!$this->isValidFilePath($value['file'])) { + return [__('"%1" is not a valid file.', $label)]; + } + $pathInfo = $this->ioFile->getPathInfo($value['file']); + $name = $pathInfo['basename'] ?? ''; + $label = $name; + } else { + $name = $value['name'] ?? ''; + } + + // Ensure we have a valid file name + if (empty($name)) { + return [__('"%1" is not a valid file.', $label)]; + } + + return ['name' => $name, 'label' => $label]; + } + + /** + * Validate file extension + * + * @param string $fileName + * @param array $rules + * @param string $label + * @return array Returns error array or empty array + */ + private function validateFileExtension(string $fileName, array $rules, string $label): array + { + $pathInfo = $this->ioFile->getPathInfo($fileName); + $extension = $pathInfo['extension'] ?? ''; + + if (empty($extension)) { + return [__('"%1" is not a valid file.', $label)]; + } + + $fileExtensions = ArrayObjectSearch::getArrayElementByName($rules, 'file_extensions'); if ($fileExtensions !== null) { $extensions = explode(',', $fileExtensions); $extensions = array_map('trim', $extensions); @@ -209,31 +277,121 @@ protected function _validateByRules($value) } } - /** - * Check protected file extension - */ + // Check protected file extension if (!$this->_fileValidator->isValid($extension)) { return $this->_fileValidator->getMessages(); } - if (!$this->_isUploadedFile($value['tmp_name'])) { + return []; + } + + /** + * Get and validate file path + * + * @param array $value + * @param string $label + * @return string|array Returns file path or error array + */ + private function getFilePath(array $value, string $label) + { + $filePath = $value['tmp_name'] ?? $value['file'] ?? null; + if (empty($filePath)) { return [__('"%1" is not a valid file.', $label)]; } - $maxFileSize = ArrayObjectSearch::getArrayElementByName( - $rules, - 'max_file_size' - ); - if ($maxFileSize !== null) { - $size = $value['size']; - if ($maxFileSize < $size) { - return [__('"%1" exceeds the allowed file size.', $label)]; - } + if (!$this->_isUploadedFile($filePath)) { + return [__('"%1" is not a valid file.', $label)]; + } + + return $filePath; + } + + /** + * Validate file size + * + * @param array $value + * @param array $rules + * @param string $label + * @return array Returns error array or empty array + */ + private function validateFileSize(array $value, array $rules, string $label): array + { + $maxFileSize = ArrayObjectSearch::getArrayElementByName($rules, 'max_file_size'); + if ($maxFileSize === null) { + return []; + } + + $size = $value['size'] ?? 0; + // For UI component uploads, get file size if not provided + if ($size === 0 && !empty($value['file'])) { + $size = $this->getTemporaryFileSize($value['file']); + } + + if ($maxFileSize < $size) { + return [__('"%1" exceeds the allowed file size.', $label)]; } return []; } + /** + * Validate file path for security + * + * @param string $filePath + * @return bool + */ + private function isValidFilePath(string $filePath): bool + { + // Check for null bytes + if (strpos($filePath, "\0") !== false) { + return false; + } + + // Check for path traversal sequences + if (preg_match('#(^|/)\.\.(?:/|$)#', $filePath)) { + return false; + } + + // Check for Windows absolute paths + if (preg_match('#^[a-zA-Z]:[\\\\/]#', $filePath)) { + return false; + } + + // Check for backslashes at the start + if (isset($filePath[0]) && $filePath[0] === '\\') { + return false; + } + + return true; + } + + /** + * Get file size from temporary directory + * + * @param string $filePath + * @return int + */ + private function getTemporaryFileSize(string $filePath): int + { + if (!$this->isValidFilePath($filePath)) { + return 0; + } + + $pathInfo = $this->ioFile->getPathInfo($filePath); + $fileName = $pathInfo['basename'] ?? ''; + if (empty($fileName)) { + return 0; + } + + $temporaryFile = FileProcessor::TMP_DIR . '/' . ltrim($fileName, '/'); + if ($this->fileProcessor->isExist($temporaryFile)) { + $stat = $this->fileProcessor->getStat($temporaryFile); + return (int)($stat['size'] ?? 0); + } + + return 0; + } + /** * Helper function that checks if the file was uploaded. * @@ -274,7 +432,8 @@ public function validateValue($value) $label = $attribute->getStoreLabel(); $toDelete = !empty($value['delete']) ? true : false; - $toUpload = !empty($value['tmp_name']) ? true : false; + // Check both tmp_name (traditional upload) and file (UI component upload) + $toUpload = !empty($value['tmp_name']) || (!empty($value['file']) && $value['file'] !== $this->_value); if (!$toUpload && !$toDelete && $this->_value) { return true; diff --git a/app/code/Magento/Customer/Model/Metadata/Form/Image.php b/app/code/Magento/Customer/Model/Metadata/Form/Image.php index f87540c5bd9dc..25c39d3ff5d4d 100644 --- a/app/code/Magento/Customer/Model/Metadata/Form/Image.php +++ b/app/code/Magento/Customer/Model/Metadata/Form/Image.php @@ -136,27 +136,151 @@ public function __construct( */ protected function _validateByRules($value) { - $label = $value['name']; + $label = $value['name'] ?? $value['file'] ?? ''; $rules = $this->getAttribute()->getValidationRules(); + // Extract and validate file name + $fileName = $this->extractAndValidateFileName($value, $label); + if (is_array($fileName)) { + return $fileName; + } + $value['name'] = $fileName; + $label = $fileName; + + // Get and validate file path, then validate image properties + $validationResult = $this->validateFilePathAndProperties($value, $label); + if (is_array($validationResult) && isset($validationResult['error'])) { + return $validationResult['error']; + } + + $filePath = $validationResult['filePath']; + $imageProp = $validationResult['imageProp']; + + // Validate image format + $formatErrors = $this->validateImageFormat($imageProp, $value, $label); + if (!empty($formatErrors)) { + return $formatErrors; + } + + // Validate size and dimensions + return $this->validateSizeAndDimensions($value, $filePath, $imageProp, $rules, $label); + } + + /** + * Validate file path and image properties + * + * @param array $value + * @param string $label + * @return array Returns array with filePath and imageProp or error array + */ + private function validateFilePathAndProperties(array $value, string $label): array + { + $filePath = $this->getValidatedFilePath($value, $label); + if (is_array($filePath)) { + return ['error' => $filePath]; + } + + $imageProp = $this->validateImageProperties($filePath, $label); + if (is_array($imageProp) && !isset($imageProp[0])) { + return ['error' => $imageProp]; + } + + return ['filePath' => $filePath, 'imageProp' => $imageProp]; + } + + /** + * Extract and validate file name from value + * + * @param array $value + * @param string $label + * @return string|array Returns file name or error array + */ + private function extractAndValidateFileName(array $value, string $label) + { + if (empty($value['name']) && !empty($value['file'])) { + if (!$this->isValidFilePath($value['file'])) { + return [__('"%1" is not a valid file.', $label)]; + } + $pathInfo = $this->ioFileSystem->getPathInfo($value['file']); + return $pathInfo['basename'] ?? ''; + } + return $value['name'] ?? ''; + } + + /** + * Get and validate file path + * + * @param array $value + * @param string $label + * @return string|array Returns file path or error array + */ + private function getValidatedFilePath(array $value, string $label) + { + $filePath = $value['tmp_name'] ?? null; + + if (empty($filePath) && !empty($value['file'])) { + $tmpFileName = ltrim($value['file'], '/'); + + if (!$this->isValidFilePath($tmpFileName)) { + return [__('"%1" is not a valid file.', $label)]; + } + + if ($tmpFileName !== '') { + $filePath = $this->mediaEntityTmpReadDirectory->getAbsolutePath($tmpFileName); + } + } + + if (empty($filePath) || !is_string($filePath)) { + return [__('"%1" is not a valid file.', $label)]; + } + + if (!$this->mediaWriteDirectory->getDriver()->isExists($filePath)) { + return [__('"%1" is not a valid file.', $label)]; + } + + return $filePath; + } + + /** + * Validate image properties using getimagesize + * + * @param string $filePath + * @param string $label + * @return array|false Returns image properties or error array + */ + private function validateImageProperties(string $filePath, string $label) + { try { // phpcs:ignore Magento2.Functions.DiscouragedFunction - $imageProp = getimagesize($value['tmp_name']); + $imageProp = getimagesize($filePath); } catch (\Throwable $e) { $imageProp = false; } - if (!$this->_isUploadedFile($value['tmp_name']) || !$imageProp) { + if (!$this->_isUploadedFile($filePath) || !$imageProp) { return [__('"%1" is not a valid file.', $label)]; } + return $imageProp; + } + + /** + * Validate image format + * + * @param array $imageProp + * @param array $value + * @param string $label + * @return array Returns error array or empty array + */ + private function validateImageFormat(array $imageProp, array &$value, string $label): array + { $allowImageTypes = [1 => 'gif', 2 => 'jpg', 3 => 'png']; - if (!isset($allowImageTypes[$imageProp[2]])) { + if (!isset($imageProp[2]) || !isset($allowImageTypes[$imageProp[2]])) { return [__('"%1" is not a valid image format.', $label)]; } - // modify image name + // Modify image name if extension doesn't match $extension = $this->ioFileSystem->getPathInfo($value['name'])['extension']; if ($extension != $allowImageTypes[$imageProp[2]]) { $value['name'] = $this->ioFileSystem->getPathInfo($value['name'])['filename'] @@ -164,43 +288,100 @@ protected function _validateByRules($value) . $allowImageTypes[$imageProp[2]]; } - $maxFileSize = ArrayObjectSearch::getArrayElementByName( - $rules, - 'max_file_size' - ); + return []; + } + + /** + * Validate file size and image dimensions + * + * @param array $value + * @param string $filePath + * @param array $imageProp + * @param array $rules + * @param string $label + * @return array Returns error array + * @SuppressWarnings(PHPMD.CyclomaticComplexity) + */ + private function validateSizeAndDimensions( + array $value, + string $filePath, + array $imageProp, + array $rules, + string $label + ): array { $errors = []; + + $maxFileSize = ArrayObjectSearch::getArrayElementByName($rules, 'max_file_size'); if ($maxFileSize !== null) { - $size = $value['size']; + $size = $value['size'] ?? 0; + if ($size === 0 && !empty($filePath)) { + $size = $this->getFileSize($filePath); + } if ($maxFileSize < $size) { $errors[] = __('"%1" exceeds the allowed file size.', $label); } } - $maxImageWidth = ArrayObjectSearch::getArrayElementByName( - $rules, - 'max_image_width' - ); - if ($maxImageWidth !== null) { - if ($maxImageWidth < $imageProp[0]) { - $r = $maxImageWidth; - $errors[] = __('"%1" width exceeds allowed value of %2 px.', $label, $r); - } + $maxImageWidth = ArrayObjectSearch::getArrayElementByName($rules, 'max_image_width'); + if ($maxImageWidth !== null && $maxImageWidth < $imageProp[0]) { + $errors[] = __('"%1" width exceeds allowed value of %2 px.', $label, $maxImageWidth); } - $maxImageHeight = ArrayObjectSearch::getArrayElementByName( - $rules, - 'max_image_height' - ); - if ($maxImageHeight !== null) { - if ($maxImageHeight < $imageProp[1]) { - $r = $maxImageHeight; - $errors[] = __('"%1" height exceeds allowed value of %2 px.', $label, $r); - } + $maxImageHeight = ArrayObjectSearch::getArrayElementByName($rules, 'max_image_height'); + if ($maxImageHeight !== null && $maxImageHeight < $imageProp[1]) { + $errors[] = __('"%1" height exceeds allowed value of %2 px.', $label, $maxImageHeight); } return $errors; } + /** + * Get file size safely + * + * @param string $filePath + * @return int + */ + private function getFileSize(string $filePath): int + { + try { + $stat = $this->mediaWriteDirectory->getDriver()->stat($filePath); + return (int)($stat['size'] ?? 0); + } catch (\Exception $e) { + return 0; + } + } + + /** + * Validate file path for security + * + * @param string $filePath + * @return bool + */ + private function isValidFilePath(string $filePath): bool + { + // Check for null bytes + if (strpos($filePath, "\0") !== false) { + return false; + } + + // Check for path traversal sequences + if (preg_match('#(^|/)\.\.(?:/|$)#', $filePath)) { + return false; + } + + // Check for Windows absolute paths + if (preg_match('#^[a-zA-Z]:[\\\\/]#', $filePath)) { + return false; + } + + // Check for backslashes at the start + if (isset($filePath[0]) && $filePath[0] === '\\') { + return false; + } + + return true; + } + /** * Process file uploader UI component data * diff --git a/app/code/Magento/Customer/Test/Unit/Model/Metadata/Form/ImageTest.php b/app/code/Magento/Customer/Test/Unit/Model/Metadata/Form/ImageTest.php index fd13c7da02fbb..657774e977d16 100644 --- a/app/code/Magento/Customer/Test/Unit/Model/Metadata/Form/ImageTest.php +++ b/app/code/Magento/Customer/Test/Unit/Model/Metadata/Form/ImageTest.php @@ -193,9 +193,9 @@ public function testValidateIsNotValidFile() ->method('getStoreLabel') ->willReturn('File Input Field Label'); - $this->fileProcessorMock->expects($this->once()) - ->method('isExist') - ->with(FileProcessor::TMP_DIR . '/' . $value['tmp_name']) + $this->driverMock->expects($this->once()) + ->method('isExists') + ->with($value['tmp_name']) ->willReturn(true); $this->ioFileSystemMock->expects($this->once()) @@ -203,12 +203,15 @@ public function testValidateIsNotValidFile() ->willReturn($value); $model = $this->initialize([ - 'value' => $value, + 'value' => null, 'isAjax' => false, 'entityTypeCode' => CustomerMetadataInterface::ENTITY_TYPE_CUSTOMER, ]); - $this->assertEquals(['"realFileName" is not a valid file.'], $model->validateValue($value)); + $result = $model->validateValue($value); + $this->assertIsArray($result); + $this->assertCount(1, $result); + $this->assertEquals('"realFileName" is not a valid image format.', (string)$result[0]); } /** @@ -232,9 +235,19 @@ public function testValidate() ->method('getStoreLabel') ->willReturn('File Input Field Label'); + $this->attributeMetadataMock->expects($this->once()) + ->method('getValidationRules') + ->willReturn([]); + + $this->driverMock->expects($this->once()) + ->method('isExists') + ->with($value['tmp_name']) + ->willReturn(true); + + // Mock fileProcessor->isExist to make _isUploadedFile return true $this->fileProcessorMock->expects($this->once()) ->method('isExist') - ->with(FileProcessor::TMP_DIR . '/' . $value['name']) + ->with(FileProcessor::TMP_DIR . '/' . $value['basename']) ->willReturn(true); $model = $this->initialize([ @@ -277,9 +290,15 @@ public function testValidateMaxFileSize() ->method('getValidationRules') ->willReturn([$validationRuleMock]); + $this->driverMock->expects($this->once()) + ->method('isExists') + ->with($value['tmp_name']) + ->willReturn(true); + + // Mock fileProcessor->isExist to make _isUploadedFile return true $this->fileProcessorMock->expects($this->once()) ->method('isExist') - ->with(FileProcessor::TMP_DIR . '/' . $value['name']) + ->with(FileProcessor::TMP_DIR . '/' . $value['basename']) ->willReturn(true); $this->ioFileSystemMock->expects($this->any()) @@ -292,7 +311,10 @@ public function testValidateMaxFileSize() 'entityTypeCode' => CustomerMetadataInterface::ENTITY_TYPE_CUSTOMER, ]); - $this->assertEquals(['"logo.gif" exceeds the allowed file size.'], $model->validateValue($value)); + $result = $model->validateValue($value); + $this->assertIsArray($result); + $this->assertCount(1, $result); + $this->assertEquals('"logo.gif" exceeds the allowed file size.', (string)$result[0]); } /** @@ -329,18 +351,31 @@ public function testValidateMaxImageWidth() ->method('getValidationRules') ->willReturn([$validationRuleMock]); + $this->driverMock->expects($this->once()) + ->method('isExists') + ->with($value['tmp_name']) + ->willReturn(true); + + // Mock fileProcessor->isExist to make _isUploadedFile return true $this->fileProcessorMock->expects($this->once()) ->method('isExist') - ->with(FileProcessor::TMP_DIR . '/' . $value['name']) + ->with(FileProcessor::TMP_DIR . '/' . $value['basename']) ->willReturn(true); + $this->ioFileSystemMock->expects($this->any()) + ->method('getPathInfo') + ->willReturn($value); + $model = $this->initialize([ 'value' => $value, 'isAjax' => false, 'entityTypeCode' => CustomerMetadataInterface::ENTITY_TYPE_CUSTOMER, ]); - $this->assertEquals(['"logo.gif" width exceeds allowed value of 1 px.'], $model->validateValue($value)); + $result = $model->validateValue($value); + $this->assertIsArray($result); + $this->assertCount(1, $result); + $this->assertEquals('"logo.gif" width exceeds allowed value of 1 px.', (string)$result[0]); } /** @@ -373,9 +408,15 @@ public function testValidateMaxImageHeight() ->method('getValidationRules') ->willReturn([$validationRuleMock]); + $this->driverMock->expects($this->once()) + ->method('isExists') + ->with($value['tmp_name']) + ->willReturn(true); + + // Mock fileProcessor->isExist to make _isUploadedFile return true $this->fileProcessorMock->expects($this->once()) ->method('isExist') - ->with(FileProcessor::TMP_DIR . '/' . $value['name']) + ->with(FileProcessor::TMP_DIR . '/' . $value['basename']) ->willReturn(true); $this->ioFileSystemMock->expects($this->any()) @@ -388,7 +429,10 @@ public function testValidateMaxImageHeight() 'entityTypeCode' => CustomerMetadataInterface::ENTITY_TYPE_CUSTOMER, ]); - $this->assertEquals(['"logo.gif" height exceeds allowed value of 1 px.'], $model->validateValue($value)); + $result = $model->validateValue($value); + $this->assertIsArray($result); + $this->assertCount(1, $result); + $this->assertEquals('"logo.gif" height exceeds allowed value of 1 px.', (string)$result[0]); } /**