Fixed #7920 - "File contains dangerous PHP" #8666

arek1-618 · 2018-11-22T14:12:38Z

Fixed #7920 - "File contains dangerous PHP"

mariuszkrzaczkowski · 2018-11-23T14:27:40Z

app/Fields/File.php

 			if (($type && $type === 'image') || $this->getShortMimeType(0) === 'image') {
 				$this->validateImage();
+				$this->validateCodeInjectionInMetadata();
+			} else {
+				$this->validateCodeInjection();


mariuszkrzaczkowski · 2018-11-23T14:28:34Z

app/Fields/File.php

+				$returnVal = false;
+			}
+		} else {
+			if (@imagecreatefromstring($this->getContents()) === false) {


mariuszkrzaczkowski · 2018-11-23T14:37:04Z

app/Fields/File.php

+			$img->clear();
+			$img->destroy();
+		} else {
+			$img = @\imagecreatefromstring(\file_get_contents($fileImgIn));


mariuszkrzaczkowski · 2018-11-23T14:37:07Z

app/Fields/File.php

+		} else {
+			$img = @\imagecreatefromstring(\file_get_contents($fileImgIn));
+			if ($img === false) {
+				throw new \App\Exceptions\AppException('Wrong file type');


mariuszkrzaczkowski · 2018-12-07T09:49:15Z

app/Fields/File.php

+		if (extension_loaded('imagick')) {
+			try {
+				$img = new \imagick($this->path);
+				$img->valid();


mariuszkrzaczkowski · 2018-12-07T09:49:52Z

app/Fields/File.php

 	}

 	/**
 	 * Validate code injection.
 	 *
 	 * @throws \Exception
 	 */
-	private function validateCodeInjection()
+	public function validateCodeInjection()


Suggested change

public function validateCodeInjection()

private function validateCodeInjection()

mariuszkrzaczkowski · 2018-12-07T09:52:43Z

app/Fields/File.php

+				stripos($contents, '<? ') !== false ||
+				stripos($contents, '<% ') !== false ||
+				stripos($contents, '<?xpacket') !== false
+			) {
 				throw new \App\Exceptions\AppException('ERR_FILE_PHP_CODE_INJECTION');


\App\Exceptions\DangerousFile

mariuszkrzaczkowski · 2018-12-07T09:52:54Z

app/Fields/File.php

+				(empty($imageInfo['APP1']) || strpos($imageInfo['APP1'], 'Exif') === 0) &&
+				($exifdata = exif_read_data($this->path)) && !$this->validateImageMetadata($exifdata)
+			) {
+				throw new \App\Exceptions\AppException('ERR_FILE_PHP_CODE_INJECTION');


\App\Exceptions\DangerousFile

mariuszkrzaczkowski · 2018-12-07T09:53:02Z

app/Fields/File.php

@@ -420,29 +444,56 @@ private function validateImage()
 		if (preg_match('[\x01-\x08\x0c-\x1f]', $this->getContents())) {
 			throw new \App\Exceptions\AppException('ERR_FILE_WRONG_IMAGE');
 		}
+		$this->validateCodeInjectionInMetadata();
+		if (!$this->validateImageContent()) {
+			throw new \App\Exceptions\AppException('ERR_FILE_WRONG_IMAGE ||' . $this->validateError);


\App\Exceptions\DangerousFile

mariuszkrzaczkowski · 2018-12-07T09:53:07Z

app/Fields/File.php

 			}
 			if ($type && $this->getShortMimeType(0) !== $type) {
-				throw new \App\Exceptions\AppException('Wrong file type');
+				throw new \App\Exceptions\AppException('ERR_FILE_ILLEGAL_FORMAT');


\App\Exceptions\DangerousFile

mariuszkrzaczkowski · 2018-12-07T09:54:46Z

app/Fields/File.php

+				$img = new \imagick($fileImgIn);
+				$img->stripImage();
+				$img->setImageCompression(\Imagick::COMPRESSION_JPEG);
+				$img->setImageCompressionQuality(80);


Suggested change

$img->setImageCompressionQuality(80);

$img->setImageCompressionQuality(99);

mariuszkrzaczkowski · 2018-12-07T09:55:36Z

app/Fields/File.php

 				$file = static::loadFromRequest($fileDetails);
 				if (!$file->validate($type)) {
-					$attach[] = ['name' => $file->getName(), 'error' => $file->validateError, 'hash' => $request->getByType('hash', 'Text')];
-					continue;
+					if (!static::removeForbiddenTags($file->getPath(), $file->getPath())) {


Suggested change

if (!static::removeForbiddenTags($file->getPath(), $file->getPath())) {

if (!static::removeForbiddenTags($file->getPath())) {

mariuszkrzaczkowski · 2018-12-07T09:56:42Z

app/Fields/File.php

@@ -985,10 +1036,19 @@ public static function uploadAndSave(\App\Request $request, array $files, string
 		$attach = [];
 		foreach (static::transform($files, true) as $key => $transformFiles) {
 			foreach ($transformFiles as $fileDetails) {
+				$infoFile = '';


Suggested change

$infoFile = '';

$additionalNotes = '';

mariuszkrzaczkowski · 2018-12-07T09:56:53Z

app/Fields/File.php

@@ -1006,7 +1066,8 @@ public static function uploadAndSave(\App\Request $request, array $files, string
 						'name' => $file->getName(),
 						'size' => \vtlib\Functions::showBytes($file->getSize()),
 						'key' => $key,
-						'hash' => $request->getByType('hash', 'string')
+						'hash' => $request->getByType('hash', 'string'),
+						'info' => $infoFile


Suggested change

'info' => $infoFile

'additionalNotes' => $additionalNotes

mariuszkrzaczkowski · 2018-12-07T10:02:14Z

app/Fields/File.php

+				$this->validateError = $e->getMessage();
+				$returnVal = false;
+			}
+		}


mariuszkrzaczkowski · 2018-12-07T13:46:29Z

app/Fields/File.php

 				$file = static::loadFromRequest($fileDetails);
 				if (!$file->validate($type)) {
-					$attach[] = ['name' => $file->getName(), 'error' => $file->validateError, 'hash' => $request->getByType('hash', 'Text')];
-					continue;
+					if (!static::removeForbiddenTags($file->getPath())) {


Suggested change

if (!static::removeForbiddenTags($file->getPath())) {

if (!static::removeForbiddenTags($file)) {

mariuszkrzaczkowski · 2018-12-07T13:46:56Z

app/Fields/File.php

+	 *
+	 * @return bool
+	 */
+	public static function removeForbiddenTags(string $fileImgIn): bool


Suggested change

public static function removeForbiddenTags(string $fileImgIn): bool

public static function removeForbiddenTags(string $file): bool

mariuszkrzaczkowski · 2018-12-07T13:47:21Z

app/Fields/File.php

+		$result = false;
+		if (extension_loaded('imagick')) {
+			try {
+				$img = new \imagick($fileImgIn);


Suggested change

$img = new \imagick($fileImgIn);

$img = new \imagick($file->getPath());

mariuszkrzaczkowski · 2018-12-07T13:47:35Z

app/Fields/File.php

+			try {
+				$img = new \imagick($fileImgIn);
+				$img->stripImage();
+				switch (strtolower(pathinfo($fileImgIn, PATHINFO_EXTENSION))) {


Suggested change

switch (strtolower(pathinfo($fileImgIn, PATHINFO_EXTENSION))) {

switch ($file->getExtension()) {

mariuszkrzaczkowski · 2018-12-07T13:48:04Z

app/Fields/File.php

+		} else {
+			$img = \imagecreatefromstring(\file_get_contents($fileImgIn));
+			if (false !== $img) {
+				switch (strtolower(pathinfo($fileImgIn, PATHINFO_EXTENSION))) {


Suggested change

switch (strtolower(pathinfo($fileImgIn, PATHINFO_EXTENSION))) {

switch ($file->getExtension()) {

mariuszkrzaczkowski · 2018-12-07T13:50:32Z

languages/pl_pl/Settings/ConfReport.json

@@ -169,6 +169,7 @@
    "LBL_HELP_LDAP": "Protok\u00f3\u0142 u\u017cywany do uzyskiwania dost\u0119pu do baz danych przechowuj\u0105cych informacje w strukturze drzewa.",
    "LBL_HELP_OPCACHE": "Poprawia wydajno\u015b\u0107 poprzez przechowywanie skompilowanego kodu bajtowego w pami\u0119ci wsp\u00f3\u0142dzielonej.",
    "LBL_HELP_APCU": "",
+    "LBL_HELP_IMAGICK": "Zaawansowana obr\u00f3bka zdje\u0107",


Biblioteka zalecana do zabezpieczania potencjalnie niebezpiecznych plików graficznych

mariuszkrzaczkowski · 2018-12-07T13:51:37Z

languages/pl_pl/_Base.json

@@ -227,6 +227,7 @@
    "LBL_EXPORT_RECORDS": "Eksportuj rekordy",
    "LBL_EXPORT_SELECTED_RECORDS": "Eksportuj zaznaczone rekordy",
    "LBL_Feb": "Lut",
+    "LBL_FILE_HAS_BEEN_MODIFIED": "Tw\u00f3j plik zosta\u0142 zmodyfikowany.",


Plik został zmodyfikowany ponieważ zawierał niebezpieczny kod.

YetiForceCompany#7920 Fixed "File contains dangerous PHP"

829efcf

arek1-618 added the ▶ in progress We’re currently working on this matter. label Nov 22, 2018

arek1-618 added this to the YetiForce 5.0 milestone Nov 22, 2018

Arkadiusz Adach added 2 commits November 23, 2018 09:39

Protection against code injection

ac33777

Protection against code injection

70c5c61

arek1-618 requested a review from mariuszkrzaczkowski November 23, 2018 09:27

arek1-618 added ✔ finished This pull request has been finished. and removed ▶ in progress We’re currently working on this matter. labels Nov 23, 2018

arek1-618 assigned mariuszkrzaczkowski and rskrzypczak Nov 23, 2018

arek1-618 requested a review from rskrzypczak November 23, 2018 09:28

Addition of messages to the test

7e52568

mariuszkrzaczkowski requested changes Nov 23, 2018

View reviewed changes

rskrzypczak added 🔧 AmendmentsRequired This pull request requires some amendments in order to be merged. and removed ✔ finished This pull request has been finished. labels Nov 23, 2018

rskrzypczak assigned arek1-618 and unassigned mariuszkrzaczkowski and rskrzypczak Nov 23, 2018

Arkadiusz Adach added 2 commits November 29, 2018 13:52

Changing the image validation method

ca3408f

Changing the image validation method

e5d4da9

arek1-618 added ✔ finished This pull request has been finished. and removed 🔧 AmendmentsRequired This pull request requires some amendments in order to be merged. labels Nov 29, 2018

arek1-618 assigned mariuszkrzaczkowski and unassigned arek1-618 Nov 29, 2018

mariuszkrzaczkowski added 🔧 AmendmentsRequired This pull request requires some amendments in order to be merged. and removed ✔ finished This pull request has been finished. labels Nov 30, 2018

Arkadiusz Adach added 3 commits December 5, 2018 12:13

Corrections to the file validation mechanism

24e28fc

Corrections to the file validation mechanism

3f33015

Corrections to the file validation mechanism

017fe60

arek1-618 added the ✔ finished This pull request has been finished. label Dec 5, 2018

arek1-618 removed the 🔧 AmendmentsRequired This pull request requires some amendments in order to be merged. label Dec 5, 2018

Removal of unused code

1fe3d25

mariuszkrzaczkowski reviewed Dec 7, 2018

View reviewed changes

app/Fields/File.php Outdated

if (extension_loaded('imagick')) {

try {

$img = new \imagick($this->path);

$img->valid();

Copy link

Member

mariuszkrzaczkowski Dec 7, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

?

mariuszkrzaczkowski reviewed Dec 7, 2018

View reviewed changes

mariuszkrzaczkowski added 🔧 AmendmentsRequired This pull request requires some amendments in order to be merged. and removed ✔ finished This pull request has been finished. labels Dec 7, 2018

mariuszkrzaczkowski reviewed Dec 7, 2018

View reviewed changes

app/Fields/File.php

$this->validateError = $e->getMessage();

$returnVal = false;

}

}

Copy link

Member

mariuszkrzaczkowski Dec 7, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

?

Improving the file validation mechanism

714800d

arek1-618 added ✔ finished This pull request has been finished. and removed 🔧 AmendmentsRequired This pull request requires some amendments in order to be merged. labels Dec 7, 2018

Arkadiusz Adach and others added 3 commits December 7, 2018 14:33

Clean code

67d53fe

Update File.php

9c80d20

Update File.php

e8cb91f

mariuszkrzaczkowski requested changes Dec 7, 2018

View reviewed changes

mariuszkrzaczkowski added 🔧 AmendmentsRequired This pull request requires some amendments in order to be merged. and removed ✔ finished This pull request has been finished. labels Dec 7, 2018

Arkadiusz Adach added 2 commits December 7, 2018 15:12

Clean code

ba9d269

Clean code

dfc103e

arek1-618 added ✔ finished This pull request has been finished. and removed 🔧 AmendmentsRequired This pull request requires some amendments in order to be merged. labels Dec 7, 2018

mariuszkrzaczkowski merged commit dfc103e into YetiForceCompany:developer Dec 10, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixed #7920 - "File contains dangerous PHP" #8666

Fixed #7920 - "File contains dangerous PHP" #8666

arek1-618 commented Nov 22, 2018

mariuszkrzaczkowski Nov 23, 2018

mariuszkrzaczkowski Nov 23, 2018

mariuszkrzaczkowski Nov 23, 2018

mariuszkrzaczkowski Nov 23, 2018

mariuszkrzaczkowski Dec 7, 2018

mariuszkrzaczkowski Dec 7, 2018

mariuszkrzaczkowski Dec 7, 2018

mariuszkrzaczkowski Dec 7, 2018

mariuszkrzaczkowski Dec 7, 2018

mariuszkrzaczkowski Dec 7, 2018

mariuszkrzaczkowski Dec 7, 2018

mariuszkrzaczkowski Dec 7, 2018

mariuszkrzaczkowski Dec 7, 2018

mariuszkrzaczkowski Dec 7, 2018

mariuszkrzaczkowski Dec 7, 2018

mariuszkrzaczkowski Dec 7, 2018

mariuszkrzaczkowski Dec 7, 2018

mariuszkrzaczkowski Dec 7, 2018

mariuszkrzaczkowski Dec 7, 2018

mariuszkrzaczkowski Dec 7, 2018

mariuszkrzaczkowski Dec 7, 2018

mariuszkrzaczkowski Dec 7, 2018

	public function validateCodeInjection()
	private function validateCodeInjection()

	$img->setImageCompressionQuality(80);
	$img->setImageCompressionQuality(99);

	if (!static::removeForbiddenTags($file->getPath(), $file->getPath())) {
	if (!static::removeForbiddenTags($file->getPath())) {

	if (!static::removeForbiddenTags($file->getPath())) {
	if (!static::removeForbiddenTags($file)) {

	public static function removeForbiddenTags(string $fileImgIn): bool
	public static function removeForbiddenTags(string $file): bool

	$img = new \imagick($fileImgIn);
	$img = new \imagick($file->getPath());

	switch (strtolower(pathinfo($fileImgIn, PATHINFO_EXTENSION))) {
	switch ($file->getExtension()) {

Fixed #7920 - "File contains dangerous PHP" #8666

Fixed #7920 - "File contains dangerous PHP" #8666

Conversation

arek1-618 commented Nov 22, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment