Issues in ImageProcessor.php (master) - Issues in master - nyacide/amma - Measure and Improve Code Quality continuously with Scrutinizer

Issues (415)

Security Analysis not enabled

Cross-Site Scripting

Cross-Site Scripting enables an attacker to inject code into the response of a web-request that is viewed by other users. It can for example be used to bypass access controls, or even to take over other users' accounts.

File Exposure

File Exposure allows an attacker to gain access to local files that he should not be able to access. These files can for example include database credentials, or other configuration files.

File Manipulation

File Manipulation enables an attacker to write custom data to files. This potentially leads to injection of arbitrary code on the server.

Object Injection

Object Injection enables an attacker to inject an object into PHP code, and can lead to arbitrary code execution, file exposure, or file manipulation attacks.

Code Injection

Code Injection enables an attacker to execute arbitrary code on the server.

Response Splitting

Response Splitting can be used to send arbitrary responses.

File Inclusion

File Inclusion enables an attacker to inject custom files into PHP's file loading mechanism, either explicitly passed to include, or for example via PHP's auto-loading mechanism.

Command Injection

Command Injection enables an attacker to inject a shell command that is execute with the privileges of the web-server. This can be used to expose sensitive data, or gain access of your server.

SQL Injection

SQL Injection enables an attacker to execute arbitrary SQL code on your database server gaining access to user data, or manipulating user data.

XPath Injection

XPath Injection enables an attacker to modify the parts of XML document that are read. If that XML document is for example used for authentication, this can lead to further vulnerabilities similar to SQL Injection.

LDAP Injection

LDAP Injection enables an attacker to inject LDAP statements potentially granting permission to run unauthorized queries, or modify content inside the LDAP tree.

Header Injection

Other Vulnerability

This category comprises other attack vectors such as manipulating the PHP runtime, loading custom extensions, freezing the runtime, or similar.

Regex Injection

Regex Injection enables an attacker to execute arbitrary code in your PHP process.

XML Injection

XML Injection enables an attacker to read files on your local filesystem including configuration files, or can be abused to freeze your web-server process.

Variable Injection

Variable Injection enables an attacker to overwrite program variables with custom data, and can lead to further vulnerabilities.

Unfortunately, the security analysis is currently not available for your project. If you are a non-commercial open-source project, please contact support to gain access.

app/Services/ImageProcessor.php (9 issues)

Labels

Severity

Upgrade to new PHP Analysis Engine

These results are based on our legacy PHP analysis, consider migrating to our new PHP analysis engine instead. Learn more

<?php

namespace App\Services;

use App\Image;
use Exception;
use Illuminate\Database\Eloquent\Model;
use Illuminate\Http\UploadedFile;

class ImageProcessor
{
    const MIME_JPEG = 'image/jpeg';
    const MIME_JPG  = 'image/jpg';
    const MIME_PNG  = 'image/png';
    const MIME_GIF  = 'image/gif';

    protected $location = 'upload/images';

    protected $filename;
    
    /**
     * @return Image
     */
    public function getModel()
    {
        return new Image();
    }

    /**
     * Upload and create image for imageable instance.
     *
     * @param $image
     * @param $imageable
     * @param null $data
     * @param null $location
     * @return static
     * @throws Exception
     */
    public function uploadAndCreate($image, $imageable, $data = null, $location = null)
    {
        if($this->validateImage($image)) {
            $original = $image->getClientOriginalName();
            if (isset($location))
                $this->setLocation($location);

            $data['original'] = $original;

            if ($imageInfo = $this->upload($image))
            {
                return $this->create($imageInfo->getPathname(), $imageable, $data);
            }
        }
    }

    /**
     * Download image to server and get as UploadedFile object.
     *
     * @param $url
     * @param $imageable
     * @param array|null $data
     * @param string $location
     * @return static
     */
    public function downloadAndUpload($url, $imageable, array $data = null, $location = 'upload/images/')
    {
        $file = file_get_contents($url); // download the image from url.
        $extension = $this->parseExtension(getimagesize($url)['mime']);

        if (isset($location))
            $this->setLocation($location);
        $filename = time() .'_'. str_random(15);

        $path_to_image = $this->pattern($filename, $extension, $location);
        file_put_contents($path_to_image, $file);

        $imageInfo = new UploadedFile($path_to_image, str_random(10));
        $data = array_merge(['original' => $filename], $data);

        return $this->create($imageInfo->getRealPath(), $imageable, $data);
    }

    /**
     * Validate incoming image.
     *
     * @param $image
     * @return bool
     */
    private function validateImage($image)
    {
        return (bool) ($image instanceof UploadedFile);
    }

    /**
     * Render image name.
     *
     * @param $image
     * @return string
     */
    private function getNewImageName($image)
    {
        $hash = md5_file($image->getRealPath());
        $ext = $image->getClientOriginalExtension();
        $time = time();
        return "{$time}_{$hash}.{$ext}";
    }

    /**
     * @return string
     */
    public function getLocation()
    {
        return $this->location;
    }

    /**
     * @param Image $image
     */
    public function destroy($image)

    {
        if($image) {
            $location = ltrim($image->image, '/');
<?php

/**
 * @property int $x
 * @property int $y
 * @property string $text
 */
class MyLabel
{
    private $properties;

    private $allowedProperties = array('x', 'y', 'text');

    public function __get($name)
    {
        if (isset($properties[$name]) && in_array($name, $this->allowedProperties)) {
            return $properties[$name];
        } else {
            return null;
        }
    }

    public function __set($name, $value)
    {
        if (in_array($name, $this->allowedProperties)) {
            $properties[$name] = $value;
        } else {
            throw new \LogicException("Property $name is not defined.");
        }
    }

}
            @unlink(base_path("public/{$location}"));
// For example instead of
@mkdir($dir);

// Better use
if (@mkdir($dir) === false) {
    throw new \RuntimeException('The directory '.$dir.' could not be created.');
}

            $image->delete();
        }
    }

    /**
     * Delete only image form folder.
     *
     * @param $image
     * @return bool
     */
    public function destroyImageOnly($image)

    {
        if($image) {
            $location = ltrim($image->image, '/');
            return @unlink(base_path("public/{$location}"));
        }
        
        return false;
    }

    /**
     * Set location.
     *
     * @param $location
     * @return $this
     */
    public function setLocation($location)
    {
        $this->location = trim($location, '/');

        return $this;
    }

    /**
     * @param $location
     * @throws Exception
     */
    protected function validateLocation($location)
    {
        $fileExists = file_exists($location);

        if ((! $fileExists && ! mkdir($location, 0777, true)) || ! is_writeable($location)) {
            throw new Exception("Location [$location] not exists or not writable");
        }
    }

    protected function upload($image)
    {
        $location = base_path("public/" . $this->location);

        $this->validateLocation($location);

        if ($image) {
            if ($image->isValid()) {
                $filename = rtrim($location, '/') . '/' . ltrim($this->getNewImageName($image), '/');

                $this->filename = $filename;

                return $image->move($location, $filename);
            }
        }

        return false;
    }

    /**
     * Change avatar.
     *
     * @param $avatar
     * @param string $location
     * @return $this
     */
    public function changeAvatar($avatar, $location = 'upload/images/user_avatars/')
    {
        if ($old_avatar = \Auth::user()->images()->avatar()->first())
            $this->destroy($old_avatar);

        if(is_string($avatar)) { // check if url.
            $this->downloadAndUpload($avatar, \Auth::user(), ['type' => 'avatar'], $location);

            return $this;
        }

        $this->uploadAndCreate($avatar, \Auth::user(), ['type' => 'avatar'], $location);

        return $this;
    }

    /**
     * Standart pattern.
     *
     * @param $location
     * @param $filename
     * @param $extension
     * @return string
     */
    private function pattern($filename, $extension, $location = null)
    {
        if(! $location)
            return sprintf('%s.%s', $filename, $extension);

        return sprintf('%s%s.%s', $location, $filename, $extension);
    }

    /**
     * Parse extension.
     *
     * @param $mime
     * @return string
     */
    private function parseExtension($mime)
    {
        switch ($mime)
        {
            case self::MIME_JPEG:
                return 'jpeg';
                break;
switch ($x) {
    case 1:
        return 'foo';
        break; // This break is not necessary and can be left off.
}
            case self::MIME_JPG:
                return 'jpg';
                break;
switch ($x) {
    case 1:
        return 'foo';
        break; // This break is not necessary and can be left off.
}
            case self::MIME_PNG:
                return 'png';
                break;
switch ($x) {
    case 1:
        return 'foo';
        break; // This break is not necessary and can be left off.
}
            case self::MIME_GIF:
                return 'gif';
                break;
switch ($x) {
    case 1:
        return 'foo';
        break; // This break is not necessary and can be left off.
}
            default:
                return 'jpeg';
                break;
switch ($x) {
    case 1:
        return 'foo';
        break; // This break is not necessary and can be left off.
}
        }
    }

    private function create($path, $imageable, array $data = null)
    {
        return $this->getModel()->create([
            'imageable_id' => $imageable->id,
            'imageable_type' => get_class($imageable),
            'type' => isset($data['type']) ? $data['type'] : 'cover',
            'original' => isset($data['original']) ? $data['original'] : str_random(13),
            'image' => str_replace(base_path('public'), '', $path),
        ]);
    }
}

1		<?php
2
3		namespace App\Services;
4
5		use App\Image;
6		use Exception;
7		use Illuminate\Database\Eloquent\Model;
8		use Illuminate\Http\UploadedFile;
9
10		class ImageProcessor
11		{
12		const MIME_JPEG = 'image/jpeg';
13		const MIME_JPG = 'image/jpg';
14		const MIME_PNG = 'image/png';
15		const MIME_GIF = 'image/gif';
16
17		protected $location = 'upload/images';
18
19		protected $filename;
20
21		/**
22		* @return Image
23		*/
24		public function getModel()
25		{
26		return new Image();
27		}
28
29		/**
30		* Upload and create image for imageable instance.
31		*
32		* @param $image
33		* @param $imageable
34		* @param null $data
35		* @param null $location
36		* @return static
37		* @throws Exception
38		*/
39		public function uploadAndCreate($image, $imageable, $data = null, $location = null)
40		{
41		if($this->validateImage($image)) {
42		$original = $image->getClientOriginalName();
43		if (isset($location))
44		$this->setLocation($location);
45
46		$data['original'] = $original;
47
48		if ($imageInfo = $this->upload($image))
49		{
50		return $this->create($imageInfo->getPathname(), $imageable, $data);
51		}
52		}
53		}
54
55		/**
56		* Download image to server and get as UploadedFile object.
57		*
58		* @param $url
59		* @param $imageable
60		* @param array\|null $data
61		* @param string $location
62		* @return static
63		*/
64		public function downloadAndUpload($url, $imageable, array $data = null, $location = 'upload/images/')
65		{
66		$file = file_get_contents($url); // download the image from url.
67		$extension = $this->parseExtension(getimagesize($url)['mime']);
68
69		if (isset($location))
70		$this->setLocation($location);
71		$filename = time() .'_'. str_random(15);
72
73		$path_to_image = $this->pattern($filename, $extension, $location);
74		file_put_contents($path_to_image, $file);
75
76		$imageInfo = new UploadedFile($path_to_image, str_random(10));
77		$data = array_merge(['original' => $filename], $data);
78
79		return $this->create($imageInfo->getRealPath(), $imageable, $data);
80		}
81
82		/**
83		* Validate incoming image.
84		*
85		* @param $image
86		* @return bool
87		*/
88		private function validateImage($image)
89		{
90		return (bool) ($image instanceof UploadedFile);
91		}
92
93		/**
94		* Render image name.
95		*
96		* @param $image
97		* @return string
98		*/
99		private function getNewImageName($image)
100		{
101		$hash = md5_file($image->getRealPath());
102		$ext = $image->getClientOriginalExtension();
103		$time = time();
104		return "{$time}_{$hash}.{$ext}";
105		}
106
107		/**
108		* @return string
109		*/
110		public function getLocation()
111		{
112		return $this->location;
113		}
114
115		/**
116		* @param Image $image
117		*/
118	View Code Duplication	public function destroy($image)
		0 ignored issues – show Duplication introduced 2016-12-28 08:14 UTC by Report Bug Copy Issue Report Show Similar Issues like this This method seems to be duplicated in your project. Duplicated code is one of the most pungent code smells. If you need to duplicate the same code in three or more different places, we strongly encourage you to look into extracting the code into a single class or operation. You can also find more detailed suggestions in the “Code” section of your repository. Loading history...
119		{
120		if($image) {
121		$location = ltrim($image->image, '/');
		0 ignored issues – show Documentation introduced 2016-12-28 08:14 UTC by Report Bug Copy Issue Report Show Similar Issues like this The property `image` does not exist on `object<App\Image>`. Since you implemented `__get`, maybe consider adding a @property annotation. Since your code implements the magic getter `_get`, this function will be called for any read access on an undefined variable. You can add the `@property` annotation to your class or interface to document the existence of this variable. <?php /** * @property int $x * @property int $y * @property string $text */ class MyLabel { private $properties; private $allowedProperties = array('x', 'y', 'text'); public function __get($name) { if (isset($properties[$name]) && in_array($name, $this->allowedProperties)) { return $properties[$name]; } else { return null; } } public function __set($name, $value) { if (in_array($name, $this->allowedProperties)) { $properties[$name] = $value; } else { throw new \LogicException("Property $name is not defined."); } } } If the property has read access only, you can use the @property-read annotation instead. Of course, you may also just have mistyped another name, in which case you should fix the error. See also the PhpDoc documentation for @property. Loading history...
122		@unlink(base_path("public/{$location}"));
		0 ignored issues – show Security Best Practice introduced 2016-12-28 08:14 UTC by Report Bug Copy Issue Report Show Similar Issues like this It seems like you do not handle an error condition here. This can introduce security issues, and is generally not recommended. If you suppress an error, we recommend checking for the error condition explicitly: // For example instead of @mkdir($dir); // Better use if (@mkdir($dir) === false) { throw new \RuntimeException('The directory '.$dir.' could not be created.'); } Loading history...
123
124		$image->delete();
125		}
126		}
127
128		/**
129		* Delete only image form folder.
130		*
131		* @param $image
132		* @return bool
133		*/
134	View Code Duplication	public function destroyImageOnly($image)
		0 ignored issues – show Duplication introduced 2016-12-28 08:14 UTC by Report Bug Copy Issue Report Show Similar Issues like this This method seems to be duplicated in your project. Duplicated code is one of the most pungent code smells. If you need to duplicate the same code in three or more different places, we strongly encourage you to look into extracting the code into a single class or operation. You can also find more detailed suggestions in the “Code” section of your repository. Loading history...
135		{
136		if($image) {
137		$location = ltrim($image->image, '/');
138		return @unlink(base_path("public/{$location}"));
139		}
140
141		return false;
142		}
143
144		/**
145		* Set location.
146		*
147		* @param $location
148		* @return $this
149		*/
150		public function setLocation($location)
151		{
152		$this->location = trim($location, '/');
153
154		return $this;
155		}
156
157		/**
158		* @param $location
159		* @throws Exception
160		*/
161		protected function validateLocation($location)
162		{
163		$fileExists = file_exists($location);
164
165		if ((! $fileExists && ! mkdir($location, 0777, true)) \|\| ! is_writeable($location)) {
166		throw new Exception("Location [$location] not exists or not writable");
167		}
168		}
169
170		protected function upload($image)
171		{
172		$location = base_path("public/" . $this->location);
173
174		$this->validateLocation($location);
175
176		if ($image) {
177		if ($image->isValid()) {
178		$filename = rtrim($location, '/') . '/' . ltrim($this->getNewImageName($image), '/');
179
180		$this->filename = $filename;
181
182		return $image->move($location, $filename);
183		}
184		}
185
186		return false;
187		}
188
189		/**
190		* Change avatar.
191		*
192		* @param $avatar
193		* @param string $location
194		* @return $this
195		*/
196		public function changeAvatar($avatar, $location = 'upload/images/user_avatars/')
197		{
198		if ($old_avatar = \Auth::user()->images()->avatar()->first())
199		$this->destroy($old_avatar);
200
201		if(is_string($avatar)) { // check if url.
202		$this->downloadAndUpload($avatar, \Auth::user(), ['type' => 'avatar'], $location);
203
204		return $this;
205		}
206
207		$this->uploadAndCreate($avatar, \Auth::user(), ['type' => 'avatar'], $location);
208
209		return $this;
210		}
211
212		/**
213		* Standart pattern.
214		*
215		* @param $location
216		* @param $filename
217		* @param $extension
218		* @return string
219		*/
220		private function pattern($filename, $extension, $location = null)
221		{
222		if(! $location)
223		return sprintf('%s.%s', $filename, $extension);
224
225		return sprintf('%s%s.%s', $location, $filename, $extension);
226		}
227
228		/**
229		* Parse extension.
230		*
231		* @param $mime
232		* @return string
233		*/
234		private function parseExtension($mime)
235		{
236		switch ($mime)
237		{
238		case self::MIME_JPEG:
239		return 'jpeg';
240		break;
		0 ignored issues – show Unused Code introduced 2016-12-28 08:14 UTC by Report Bug Copy Issue Report Show Similar Issues like this `break` is not strictly necessary here and could be removed. The `break` statement is not necessary if it is preceded for example by a `return` statement: switch ($x) { case 1: return 'foo'; break; // This break is not necessary and can be left off. } If you would like to keep this construct to be consistent with other `case` statements, you can safely mark this issue as a false-positive. Loading history...
241		case self::MIME_JPG:
242		return 'jpg';
243		break;
		0 ignored issues – show Unused Code introduced 2016-12-28 08:14 UTC by Report Bug Copy Issue Report Show Similar Issues like this `break` is not strictly necessary here and could be removed. The `break` statement is not necessary if it is preceded for example by a `return` statement: switch ($x) { case 1: return 'foo'; break; // This break is not necessary and can be left off. } If you would like to keep this construct to be consistent with other `case` statements, you can safely mark this issue as a false-positive. Loading history...
244		case self::MIME_PNG:
245		return 'png';
246		break;
		0 ignored issues – show Unused Code introduced 2016-12-28 08:14 UTC by Report Bug Copy Issue Report Show Similar Issues like this `break` is not strictly necessary here and could be removed. The `break` statement is not necessary if it is preceded for example by a `return` statement: switch ($x) { case 1: return 'foo'; break; // This break is not necessary and can be left off. } If you would like to keep this construct to be consistent with other `case` statements, you can safely mark this issue as a false-positive. Loading history...
247		case self::MIME_GIF:
248		return 'gif';
249		break;
		0 ignored issues – show Unused Code introduced 2016-12-28 08:14 UTC by Report Bug Copy Issue Report Show Similar Issues like this `break` is not strictly necessary here and could be removed. The `break` statement is not necessary if it is preceded for example by a `return` statement: switch ($x) { case 1: return 'foo'; break; // This break is not necessary and can be left off. } If you would like to keep this construct to be consistent with other `case` statements, you can safely mark this issue as a false-positive. Loading history...
250		default:
251		return 'jpeg';
252		break;
		0 ignored issues – show Unused Code introduced 2016-12-28 08:14 UTC by Report Bug Copy Issue Report Show Similar Issues like this `break` is not strictly necessary here and could be removed. The `break` statement is not necessary if it is preceded for example by a `return` statement: switch ($x) { case 1: return 'foo'; break; // This break is not necessary and can be left off. } If you would like to keep this construct to be consistent with other `case` statements, you can safely mark this issue as a false-positive. Loading history...
253		}
254		}
255
256		private function create($path, $imageable, array $data = null)
257		{
258		return $this->getModel()->create([
259		'imageable_id' => $imageable->id,
260		'imageable_type' => get_class($imageable),
261		'type' => isset($data['type']) ? $data['type'] : 'cover',
262		'original' => isset($data['original']) ? $data['original'] : str_random(13),
263		'image' => str_replace(base_path('public'), '', $path),
264		]);
265		}
266		}

nyacide / amma

Issues (415)

Security Analysis not enabled

app/Services/ImageProcessor.php (9 issues)

Labels

Severity

Introduced By

Upgrade to new PHP Analysis Engine

Duplication Side-by-Side

Filter issues like