diff --git a/protected/humhub/docs/CHANGELOG.md b/protected/humhub/docs/CHANGELOG.md index c07d849ceb..79f8d1dd14 100644 --- a/protected/humhub/docs/CHANGELOG.md +++ b/protected/humhub/docs/CHANGELOG.md @@ -5,6 +5,8 @@ HumHub Change Log - Fix: Disabled module notification category visible in notification settings. - Enh: Added `ModuleManager::getEnabledModules()` - Enh: `LikeAsset` is now part of `AppAsset` and does not need further registration +- Fix: Reflective XSS in file post upload (thanks to **Rubal Jain** for testing and reporting) +- Enh: Added further upload file name validation 1.3.10 (February 22, 2019) --------------------------- diff --git a/protected/humhub/modules/file/Module.php b/protected/humhub/modules/file/Module.php index 443fcfef61..38c279daac 100644 --- a/protected/humhub/modules/file/Module.php +++ b/protected/humhub/modules/file/Module.php @@ -21,6 +21,11 @@ class Module extends \humhub\components\Module */ public $isCoreModule = true; + /** + * @inheritdoc + */ + public $fileNameValidationPattern = '/[\x00-\x1F\x80-\xA0>\/\<"\':\*?|{}\[\]\\\\\/]/u'; + /** * @see components\StorageManagerInterface * @var string storage manager class for files diff --git a/protected/humhub/modules/file/resources/js/humhub.file.js b/protected/humhub/modules/file/resources/js/humhub.file.js index 741de45a28..81701bcee1 100644 --- a/protected/humhub/modules/file/resources/js/humhub.file.js +++ b/protected/humhub/modules/file/resources/js/humhub.file.js @@ -359,6 +359,7 @@ humhub.module('file', function (module, require, $) { }; Preview.prototype.add = function (file) { + file.name = string.encode(file.name); file.galleryId = this.$.attr('id') + '_file_preview_gallery'; var template = this.getTemplate(file); diff --git a/protected/humhub/modules/file/tests/codeception/unit/FileValidatorTest.php b/protected/humhub/modules/file/tests/codeception/unit/FileValidatorTest.php new file mode 100644 index 0000000000..2dad90cbff --- /dev/null +++ b/protected/humhub/modules/file/tests/codeception/unit/FileValidatorTest.php @@ -0,0 +1,96 @@ +assertTrue( $this->createFile('shouldwork.jpg')->validate()); + $this->assertTrue( $this->createFile('should¡work.jpg')->validate()); + $this->assertTrue( $this->createFile('shouldÿwork.jpg')->validate()); + $this->assertTrue($this->createFile("lästig.jpg")->validate()); + $this->assertTrue($this->createFile("lä123stig.jpg")->validate()); + $this->assertTrue($this->createFile("test-this.jpg")->validate()); + $this->assertTrue($this->createFile("test_this.jpg")->validate()); + $this->assertTrue($this->createFile("test this.jpg")->validate()); + $this->assertTrue($this->createFile("test@this.jpg")->validate()); + $this->assertTrue($this->createFile("test€this.jpg")->validate()); + $this->assertTrue($this->createFile("hârt.jpg")->validate()); + $this->assertTrue($this->createFile("我.jpg")->validate()); + $this->assertTrue($this->createFile("昨夜の.jpg")->validate()); + $this->assertTrue($this->createFile("ْعَرَبِيَّة.jpg")->validate()); + $this->assertTrue($this->createFile("test.this.jpg")->validate()); + $this->assertTrue($this->createFile("çore.jpg")->validate()); + + $this->assertFalse($this->createFile("testChar\x00.jpg")->validate()); + $this->assertFalse($this->createFile("testChar\x1f.jpg")->validate()); + $this->assertFalse($this->createFile("testChar\xc2\x80.jpg")->validate()); + $this->assertFalse($this->createFile("testChar\xc2\x9f.jpg")->validate()); + + $this->assertFalse($this->createFile("testChar\fst.jpg")->validate()); + $this->assertFalse($this->createFile("testChar\nst.jpg")->validate()); + $this->assertFalse($this->createFile("testChar\rst.jpg")->validate()); + $this->assertFalse($this->createFile("testChar\tst.jpg")->validate()); + $this->assertFalse($this->createFile("testChar\0st.jpg")->validate()); + $this->assertFalse($this->createFile("testChar\1st.jpg")->validate()); + $this->assertFalse($this->createFile("testChar\2st.jpg")->validate()); + $this->assertFalse($this->createFile("testChar\3st.jpg")->validate()); + $this->assertFalse($this->createFile("testChar\4st.jpg")->validate()); + $this->assertFalse($this->createFile("testChar\5st.jpg")->validate()); + $this->assertFalse($this->createFile("testChar\6st.jpg")->validate()); + $this->assertFalse($this->createFile("testChar".chr(7)."bst.jpg")->validate()); + $this->assertFalse($this->createFile("testChar".chr(8)."bst.jpg")->validate()); + $this->assertFalse($this->createFile("testChar".chr(9)."bst.jpg")->validate()); + $this->assertFalse($this->createFile("testChar".chr(10)."bst.jpg")->validate()); + $this->assertFalse($this->createFile("testChar".chr(11)."bst.jpg")->validate()); + $this->assertFalse($this->createFile("testChar".chr(12)."bst.jpg")->validate()); + $this->assertFalse($this->createFile("testChar".chr(13)."bst.jpg")->validate()); + $this->assertFalse($this->createFile("testChar".chr(14)."bst.jpg")->validate()); + $this->assertFalse($this->createFile("testChar".chr(15)."bst.jpg")->validate()); + $this->assertFalse($this->createFile("testChar".chr(16)."bst.jpg")->validate()); + $this->assertFalse($this->createFile("testChar".chr(17)."bst.jpg")->validate()); + $this->assertFalse($this->createFile("testChar".chr(18)."bst.jpg")->validate()); + $this->assertFalse($this->createFile("testChar".chr(19)."bst.jpg")->validate()); + $this->assertFalse($this->createFile("testChar".chr(20)."bst.jpg")->validate()); + $this->assertFalse($this->createFile("testChar".chr(21)."bst.jpg")->validate()); + $this->assertFalse($this->createFile("testChar".chr(22)."bst.jpg")->validate()); + $this->assertFalse($this->createFile("testChar".chr(23)."bst.jpg")->validate()); + $this->assertFalse($this->createFile("testChar".chr(24)."bst.jpg")->validate()); + $this->assertFalse($this->createFile("testChar".chr(25)."bst.jpg")->validate()); + $this->assertFalse($this->createFile("testChar".chr(26)."bst.jpg")->validate()); + $this->assertFalse($this->createFile("testChar".chr(27)."bst.jpg")->validate()); + $this->assertFalse($this->createFile("testChar".chr(28)."bst.jpg")->validate()); + $this->assertFalse($this->createFile("testChar".chr(29)."bst.jpg")->validate()); + $this->assertFalse($this->createFile("testChar".chr(30)."bst.jpg")->validate()); + $this->assertFalse($this->createFile("testChar".chr(31)."bst.jpg")->validate()); + $this->assertFalse($this->createFile("tes|st.jpg")->validate()); + $this->assertFalse($this->createFile("tes]st.jpg")->validate()); + $this->assertFalse($this->createFile("tes[st.jpg")->validate()); + $this->assertFalse($this->createFile("tes{st.jpg")->validate()); + $this->assertFalse($this->createFile("tes}st.jpg")->validate()); + $this->assertFalse($this->createFile("tes?st.jpg")->validate()); + $this->assertFalse($this->createFile("tes:st.jpg")->validate()); + $this->assertFalse($this->createFile("tes*st.jpg")->validate()); + $this->assertFalse($this->createFile("tesvalidate()); + $this->assertFalse($this->createFile("tes>st.jpg")->validate()); + + $this->assertFalse($this->createFile(".jpg")->validate()); + $this->assertFalse($this->createFile("test.jpg.exe")->validate()); + } + + private function createFile($name) + { + $file = new FileUpload(); + $file->setUploadedFile(new UploadedFile(['name' => $name])); + return $file; + } +} \ No newline at end of file diff --git a/protected/humhub/modules/file/validators/FileValidator.php b/protected/humhub/modules/file/validators/FileValidator.php index c344cfae55..13c3e8e480 100644 --- a/protected/humhub/modules/file/validators/FileValidator.php +++ b/protected/humhub/modules/file/validators/FileValidator.php @@ -9,6 +9,7 @@ namespace humhub\modules\file\validators; use Yii; +use humhub\modules\file\models\File; use humhub\modules\file\libs\ImageConverter; /** @@ -58,11 +59,40 @@ class FileValidator extends \yii\validators\FileValidator } } + /** + * @inheritdoc + */ + public function validateAttribute($model, $attribute) + { + $this->validateFileName($model, $attribute); + parent::validateAttribute($model, $attribute); + } + + public function validateFileName($model, $attribute) + { + if($model instanceof File) { + $pattern = Yii::$app->moduleManager->getModule('file')->fileNameValidationPattern; + + if(empty($pattern)) { + return; + } + + if(preg_match($pattern, $model->file_name)) { + $this->addError($model, $attribute, Yii::t('FileModule.models_File', 'Invalid file name detected!')); + } + + if(preg_match('/\.\w{2,3}\.\w{2,3}$/', $model->file_name)) { + $this->addError($model, $attribute, Yii::t('FileModule.models_File', 'Double file extensions are not allowed!')); + } + } + } + /** * Checks memory limit if GD is used for image conversions - * + * * @param \yii\web\UploadedFile $file - * @return array|null + * @return array|null + * @throws \yii\base\Exception */ protected function checkMemoryLimit($file) {