DRCIT/snipe-it
1
0
Fork 0
mirror of https://github.com/snipe/snipe-it.git synced 2024-12-23 12:44:12 -08:00
Code Issues Actions 1 Packages Projects Releases Wiki Activity

Fixed XSS vulnerability in SVG image uploads [ch10476] (#7639)

Browse source 
* Added enshrined/svg-sanitize

* Added modular image resizing/SVG cleaning method

(This already exists in v5, so I mostly ported it forward and added the SVG sanitizer.)

* Use improved handleImages method to upload/resize/clean images

* Removed $old_image

This is handled in the ImageUpload request now
This commit is contained in:
snipe 2019-12-05 22:23:05 -08:00 committed by GitHub
parent 3f5840d390
commit e71e57f16a
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
14 changed files with 160 additions and 359 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

44
app/Http/Controllers/AccessoriesController.php
View file

@ -85,26 +85,7 @@ class AccessoriesController extends Controller
$accessory->qty = request('qty');
$accessory->user_id = Auth::user()->id;
$accessory->supplier_id = request('supplier_id');
if ($request->hasFile('image')) {
if (!config('app.lock_passwords')) {
$image = $request->file('image');
$ext = $image->getClientOriginalExtension();
$file_name = "accessory-".str_random(18).'.'.$ext;
$path = public_path('/uploads/accessories');
if ($image->getClientOriginalExtension()!='svg') {
Image::make($image->getRealPath())->resize(null, 800, function ($constraint) {
$constraint->aspectRatio();
$constraint->upsize();
})->save($path.'/'.$file_name);
} else {
$image->move($path, $file_name);
}
$accessory->image = $file_name;
}
}
$accessory = $request->handleImages($accessory,600, public_path().'/uploads/accessories');
// Was the accessory created?
@ -165,28 +146,7 @@ class AccessoriesController extends Controller
$accessory->qty = request('qty');
$accessory->supplier_id = request('supplier_id');
if ($request->hasFile('image')) {
if (!config('app.lock_passwords')) {
$image = $request->file('image');
$ext = $image->getClientOriginalExtension();
$file_name = "accessory-".str_random(18).'.'.$ext;
$path = public_path('/uploads/accessories');
if ($image->getClientOriginalExtension()!='svg') {
Image::make($image->getRealPath())->resize(null, 800, function ($constraint) {
$constraint->aspectRatio();
$constraint->upsize();
})->save($path.'/'.$file_name);
} else {
$image->move($path, $file_name);
}
if (($accessory->image) && (file_exists($path.'/'.$accessory->image))) {
unlink($path.'/'.$accessory->image);
}
$accessory->image = $file_name;
}
}
$accessory = $request->handleImages($accessory,600, public_path().'/uploads/accessories');
// Was the accessory updated?

50
app/Http/Controllers/AssetModelsController.php
View file

@ -90,23 +90,7 @@ class AssetModelsController extends Controller
$model->fieldset_id = e($request->input('custom_fieldset'));
}
if (Input::file('image')) {
$image = Input::file('image');
$file_name = str_slug($image->getClientOriginalName()) . "." . $image->getClientOriginalExtension();
$path = app('models_upload_path');
if ($image->getClientOriginalExtension()!='svg') {
Image::make($image->getRealPath())->resize(800, null, function ($constraint) {
$constraint->aspectRatio();
$constraint->upsize();
})->save($path.'/'.$file_name);
} else {
$image->move($path, $file_name);
}
$model->image = $file_name;
}
$model = $request->handleImages($model,600, public_path().'/uploads/models');
// Was it created?
if ($model->save()) {
@ -182,37 +166,7 @@ class AssetModelsController extends Controller
}
}
$old_image = $model->image;
// Set the model's image property to null if the image is being deleted
if ($request->input('image_delete') == 1) {
$model->image = null;
}
if ($request->file('image')) {
$image = $request->file('image');
$file_name = $model->id.'-'.str_slug($image->getClientOriginalName()) . "." . $image->getClientOriginalExtension();
if ($image->getClientOriginalExtension()!='svg') {
Image::make($image->getRealPath())->resize(800, null, function ($constraint) {
$constraint->aspectRatio();
$constraint->upsize();
})->save(app('models_upload_path').$file_name);
} else {
$image->move(app('models_upload_path'), $file_name);
}
$model->image = $file_name;
}
if ((($request->file('image')) && (isset($old_image)) && ($old_image!='')) || ($request->input('image_delete') == 1)) {
try {
unlink(app('models_upload_path').$old_image);
} catch (\Exception $e) {
\Log::info($e);
}
}
$model = $request->handleImages($model,600, public_path().'/uploads/models');
if ($model->save()) {
return redirect()->route("models.index")->with('success', trans('admin/models/message.update.success'));

39
app/Http/Controllers/CategoriesController.php
View file

@ -83,17 +83,7 @@ class CategoriesController extends Controller
$category->checkin_email = $request->input('checkin_email', '0');
$category->user_id = Auth::id();
if ($request->file('image')) {
$image = $request->file('image');
$file_name = str_random(25).".".$image->getClientOriginalExtension();
$path = public_path('uploads/categories/'.$file_name);
Image::make($image->getRealPath())->resize(800, null, function ($constraint) {
$constraint->aspectRatio();
$constraint->upsize();
})->save($path);
$category->image = $file_name;
}
$category = $request->handleImages($category,600, public_path().'/uploads/categories');
if ($category->save()) {
return redirect()->route('categories.index')->with('success', trans('admin/categories/message.create.success'));
@ -152,37 +142,12 @@ class CategoriesController extends Controller
$category->require_acceptance = $request->input('require_acceptance', '0');
$category->checkin_email = $request->input('checkin_email', '0');
$old_image = $category->image;
// Set the model's image property to null if the image is being deleted
if ($request->input('image_delete') == 1) {
$category->image = null;
}
if ($request->file('image')) {
$image = $request->file('image');
$file_name = $category->id.'-'.str_slug($image->getClientOriginalName()) . "." . $image->getClientOriginalExtension();
if ($image->getClientOriginalExtension()!='svg') {
Image::make($image->getRealPath())->resize(800, null, function ($constraint) {
$constraint->aspectRatio();
$constraint->upsize();
})->save(app('categories_upload_path').$file_name);
} else {
$image->move(app('categories_upload_path'), $file_name);
}
$category->image = $file_name;
}
if ((($request->file('image')) && (isset($old_image)) && ($old_image!='')) || ($request->input('image_delete') == 1)) {
try {
unlink(app('categories_upload_path').$old_image);
} catch (\Exception $e) {
\Log::info($e);
}
}
$category = $request->handleImages($category,600, public_path().'/uploads/categories');
if ($category->save()) {
// Redirect to the new category page

37
app/Http/Controllers/CompaniesController.php
View file

@ -63,16 +63,7 @@ final class CompaniesController extends Controller
$company = new Company;
$company->name = $request->input('name');
if ($request->file('image')) {
$image = $request->file('image');
$file_name = str_random(25).".".$image->getClientOriginalExtension();
$path = public_path('uploads/companies/'.$file_name);
Image::make($image->getRealPath())->resize(800, null, function ($constraint) {
$constraint->aspectRatio();
$constraint->upsize();
})->save($path);
$company->image = $file_name;
}
$company = $request->handleImages($company,600, public_path().'/uploads/companies');
if ($company->save()) {
return redirect()->route('companies.index')
@ -121,36 +112,12 @@ final class CompaniesController extends Controller
$company->name = $request->input('name');
$old_image = $company->image;
// Set the model's image property to null if the image is being deleted
if ($request->input('image_delete') == 1) {
$company->image = null;
}
if ($request->file('image')) {
$image = $request->file('image');
$file_name = $company->id.'-'.str_slug($image->getClientOriginalName()) . "." . $image->getClientOriginalExtension();
if ($image->getClientOriginalExtension()!='svg') {
Image::make($image->getRealPath())->resize(800, null, function ($constraint) {
$constraint->aspectRatio();
$constraint->upsize();
})->save(app('companies_upload_path').$file_name);
} else {
$image->move(app('companies_upload_path'), $file_name);
}
$company->image = $file_name;
}
if ((($request->file('image')) && (isset($old_image)) && ($old_image!='')) || ($request->input('image_delete') == 1)) {
try {
unlink(app('companies_upload_path').$old_image);
} catch (\Exception $e) {
\Log::info($e);
}
}
$company = $request->handleImages($company,600, public_path().'/uploads/companies');
if ($company->save()) {

24
app/Http/Controllers/ComponentsController.php
View file

@ -91,16 +91,7 @@ class ComponentsController extends Controller
$component->user_id = Auth::id();
if ($request->file('image')) {
$image = $request->file('image');
$file_name = str_random(25).".".$image->getClientOriginalExtension();
$path = public_path('uploads/components/'.$file_name);
Image::make($image->getRealPath())->resize(800, null, function ($constraint) {
$constraint->aspectRatio();
$constraint->upsize();
})->save($path);
$component->image = $file_name;
}
$component = $request->handleImages($component,600, public_path().'/uploads/components');
if ($component->save()) {
return redirect()->route('components.index')->with('success', trans('admin/components/message.create.success'));
@ -164,18 +155,7 @@ class ComponentsController extends Controller
$component->purchase_cost = request('purchase_cost');
$component->qty = Input::get('qty');
if ($request->file('image')) {
$image = $request->file('image');
$file_name = str_random(25).".".$image->getClientOriginalExtension();
$path = public_path('uploads/components/'.$file_name);
Image::make($image->getRealPath())->resize(800, null, function ($constraint) {
$constraint->aspectRatio();
$constraint->upsize();
})->save($path);
$component->image = $file_name;
} elseif ($request->input('image_delete')=='1') {
$component->image = null;
}
$component = $request->handleImages($component,600, public_path().'/uploads/components');
if ($component->save()) {
return redirect()->route('components.index')->with('success', trans('admin/components/message.update.success'));

12
app/Http/Controllers/ConsumablesController.php
View file

@ -87,16 +87,8 @@ class ConsumablesController extends Controller
$consumable->user_id = Auth::id();
if ($request->file('image')) {
$image = $request->file('image');
$file_name = str_random(25).".".$image->getClientOriginalExtension();
$path = public_path('uploads/consumables/'.$file_name);
Image::make($image->getRealPath())->resize(800, null, function ($constraint) {
$constraint->aspectRatio();
$constraint->upsize();
})->save($path);
$consumable->image = $file_name;
}
$consumable = $request->handleImages($consumable,600, public_path().'/uploads/components');
if ($consumable->save()) {
return redirect()->route('consumables.index')->with('success', trans('admin/consumables/message.create.success'));

42
app/Http/Controllers/DepartmentsController.php
View file

@ -53,16 +53,7 @@ class DepartmentsController extends Controller
$department->user_id = Auth::user()->id;
$department->manager_id = ($request->filled('manager_id' ) ? $request->input('manager_id') : null);
if ($request->file('image')) {
$image = $request->file('image');
$file_name = str_random(25).".".$image->getClientOriginalExtension();
$path = public_path('uploads/departments/'.$file_name);
Image::make($image->getRealPath())->resize(800, null, function ($constraint) {
$constraint->aspectRatio();
$constraint->upsize();
})->save($path);
$department->image = $file_name;
}
$department = $request->handleImages($department,600, public_path().'/uploads/departments');
if ($department->save()) {
return redirect()->route("departments.index")->with('success', trans('admin/departments/message.create.success'));
@ -164,36 +155,7 @@ class DepartmentsController extends Controller
$department->fill($request->all());
$department->manager_id = ($request->filled('manager_id' ) ? $request->input('manager_id') : null);
$old_image = $department->image;
// Set the model's image property to null if the image is being deleted
if ($request->input('image_delete') == 1) {
$department->image = null;
}
if ($request->file('image')) {
$image = $request->file('image');
$file_name = $department->id.'-'.str_slug($image->getClientOriginalName()) . "." . $image->getClientOriginalExtension();
if ($image->getClientOriginalExtension()!='svg') {
Image::make($image->getRealPath())->resize(800, null, function ($constraint) {
$constraint->aspectRatio();
$constraint->upsize();
})->save(app('departments_upload_path').$file_name);
} else {
$image->move(app('departments_upload_path'), $file_name);
}
$department->image = $file_name;
}
if ((($request->file('image')) && (isset($old_image)) && ($old_image!='')) || ($request->input('image_delete') == 1)) {
try {
unlink(app('departments_upload_path').$old_image);
} catch (\Exception $e) {
\Log::info($e);
}
}
$department = $request->handleImages($department,600, public_path().'/uploads/departments');
if ($department->save()) {
return redirect()->route("departments.index")->with('success', trans('admin/departments/message.update.success'));

43
app/Http/Controllers/LocationsController.php
View file

@ -88,16 +88,7 @@ class LocationsController extends Controller
$location->manager_id = $request->input('manager_id');
$location->user_id = Auth::id();
if ($request->file('image')) {
$image = $request->file('image');
$file_name = str_random(25).".".$image->getClientOriginalExtension();
$path = public_path('uploads/locations/'.$file_name);
Image::make($image->getRealPath())->resize(800, null, function ($constraint) {
$constraint->aspectRatio();
$constraint->upsize();
})->save($path);
$location->image = $file_name;
}
$location = $request->handleImages($location,600, public_path().'/uploads/locations');
if ($location->save()) {
return redirect()->route("locations.index")->with('success', trans('admin/locations/message.create.success'));
@ -162,37 +153,7 @@ class LocationsController extends Controller
$location->zip = $request->input('zip');
$location->ldap_ou = $request->input('ldap_ou');
$location->manager_id = $request->input('manager_id');
$old_image = $location->image;
// Set the model's image property to null if the image is being deleted
if ($request->input('image_delete') == 1) {
$location->image = null;
}
if ($request->file('image')) {
$image = $request->file('image');
$file_name = $location->id.'-'.str_slug($image->getClientOriginalName()) . "." . $image->getClientOriginalExtension();
if ($image->getClientOriginalExtension()!='svg') {
Image::make($image->getRealPath())->resize(800, null, function ($constraint) {
$constraint->aspectRatio();
$constraint->upsize();
})->save(app('locations_upload_path').$file_name);
} else {
$image->move(app('locations_upload_path'), $file_name);
}
$location->image = $file_name;
}
if ((($request->file('image')) && (isset($old_image)) && ($old_image!='')) || ($request->input('image_delete') == 1)) {
try {
unlink(app('locations_upload_path').$old_image);
} catch (\Exception $e) {
\Log::info($e);
}
}
$location = $request->handleImages($location,600, public_path().'/uploads/locations');
if ($location->save()) {

40
app/Http/Controllers/ManufacturersController.php
View file

@ -75,18 +75,7 @@ class ManufacturersController extends Controller
$manufacturer->support_url = $request->input('support_url');
$manufacturer->support_phone = $request->input('support_phone');
$manufacturer->support_email = $request->input('support_email');
if ($request->file('image')) {
$image = $request->file('image');
$file_name = str_slug($image->getClientOriginalName()).".".$image->getClientOriginalExtension();
$path = public_path('uploads/manufacturers/'.$file_name);
Image::make($image->getRealPath())->resize(800, null, function ($constraint) {
$constraint->aspectRatio();
$constraint->upsize();
})->save($path);
$manufacturer->image = $file_name;
}
$manufacturer = $request->handleImages($manufacturer,600, public_path().'/uploads/manufacturers');
@ -142,37 +131,14 @@ class ManufacturersController extends Controller
$manufacturer->support_url = $request->input('support_url');
$manufacturer->support_phone = $request->input('support_phone');
$manufacturer->support_email = $request->input('support_email');
$old_image = $manufacturer->image;
// Set the model's image property to null if the image is being deleted
if ($request->input('image_delete') == 1) {
$manufacturer->image = null;
}
if ($request->file('image')) {
$image = $request->file('image');
$file_name = $manufacturer->id.'-'.str_slug($image->getClientOriginalName()) . "." . $image->getClientOriginalExtension();
$manufacturer = $request->handleImages($manufacturer,600, public_path().'/uploads/manufacturers');
if ($image->getClientOriginalExtension()!='svg') {
Image::make($image->getRealPath())->resize(800, null, function ($constraint) {
$constraint->aspectRatio();
$constraint->upsize();
})->save(app('manufacturers_upload_path').$file_name);
} else {
$image->move(app('manufacturers_upload_path'), $file_name);
}
$manufacturer->image = $file_name;
}
if ((($request->file('image')) && (isset($old_image)) && ($old_image!='')) || ($request->input('image_delete') == 1)) {
try {
unlink(app('manufacturers_upload_path').$old_image);
} catch (\Exception $e) {
\Log::info($e);
}
}
if ($manufacturer->save()) {

14
app/Http/Controllers/SettingsController.php
View file

@ -1,6 +1,7 @@
<?php
namespace App\Http\Controllers;
use enshrined\svgSanitize\Sanitizer;
use Input;
use Lang;
use Illuminate\Http\Request;
@ -426,12 +427,23 @@ class SettingsController extends Controller
$file_name = "logo.".$image->getClientOriginalExtension();
$path = public_path('uploads');
if ($image->getClientOriginalExtension()!='svg') {
Image::make($image->getRealPath())->resize(null, 150, function ($constraint) {
$constraint->aspectRatio();
$constraint->upsize();
})->save($path.'/'.$file_name);
} else {
$image->move($path, $file_name);
// This is kinda copypasta from the ImageUploadRequest - should refactor the ImageUploadRequest to better handle maybe
$sanitizer = new Sanitizer();
$dirtySVG = file_get_contents($image->getRealPath());
$cleanSVG = $sanitizer->sanitize($dirtySVG);
try {
file_put_contents($path.'/'.$file_name, $cleanSVG);
} catch (\Exception $e) {
\Log::debug($e);
}
}
$setting->logo = $file_name;
}

45
app/Http/Controllers/SuppliersController.php
View file

@ -78,17 +78,8 @@ class SuppliersController extends Controller
$supplier->notes = request('notes');
$supplier->url = $supplier->addhttp(request('url'));
$supplier->user_id = Auth::id();
$supplier = $request->handleImages($supplier,600, public_path().'/uploads/suppliers');
if ($request->file('image')) {
$image = $request->file('image');
$file_name = str_random(25).".".$image->getClientOriginalExtension();
$path = public_path('uploads/suppliers/'.$file_name);
Image::make($image->getRealPath())->resize(800, null, function ($constraint) {
$constraint->aspectRatio();
$constraint->upsize();
})->save($path);
$supplier->image = $file_name;
}
if ($supplier->save()) {
return redirect()->route('suppliers.index')->with('success', trans('admin/suppliers/message.create.success'));
@ -145,39 +136,7 @@ class SuppliersController extends Controller
$supplier->email = request('email');
$supplier->url = $supplier->addhttp(request('url'));
$supplier->notes = request('notes');
$old_image = $supplier->image;
// Set the model's image property to null if the image is being deleted
if ($request->input('image_delete') == 1) {
$supplier->image = null;
}
if ($request->file('image')) {
$image = $request->file('image');
$file_name = $supplier->id.'-'.str_slug($image->getClientOriginalName()) . "." . $image->getClientOriginalExtension();
if ($image->getClientOriginalExtension()!='svg') {
Image::make($image->getRealPath())->resize(800, null, function ($constraint) {
$constraint->aspectRatio();
$constraint->upsize();
})->save(app('suppliers_upload_path').$file_name);
} else {
$image->move(app('suppliers_upload_path'), $file_name);
}
$supplier->image = $file_name;
}
if ((($request->file('image')) && (isset($old_image)) && ($old_image!='')) || ($request->input('image_delete') == 1)) {
try {
unlink(app('suppliers_upload_path').$old_image);
} catch (\Exception $e) {
\Log::info($e);
}
}
$supplier = $request->handleImages($supplier,600, public_path().'/uploads/suppliers');
if ($supplier->save()) {
return redirect()->route('suppliers.index')->with('success', trans('admin/suppliers/message.update.success'));

85
app/Http/Requests/ImageUploadRequest.php
View file

@ -2,7 +2,9 @@
namespace App\Http\Requests;
use App\Http\Requests\Request;
use App\Models\SnipeModel;
use Intervention\Image\Facades\Image;
use enshrined\svgSanitize\Sanitizer;
class ImageUploadRequest extends Request
{
@ -33,4 +35,83 @@ class ImageUploadRequest extends Request
{
return $this->redirector->back()->withInput()->withErrors($errors, $this->errorBag);
}
}
/**
* Handle and store any images attached to request
* @param SnipeModel $item Item the image is associated with
* @param String $path location for uploaded images, defaults to uploads/plural of item type.
* @return SnipeModel Target asset is being checked out to.
*/
public function handleImages($item, $w = 600, $path = null)
{
$type = strtolower(class_basename(get_class($item)));
if (is_null($path)) {
$path = str_plural($type);
}
\Log::debug('Trying to upload to '. $path);
if ($this->hasFile('image')) {
if (!config('app.lock_passwords')) {
if (!is_dir($path)) {
\Log::debug($path.' does not exist');
mkdir($path);
}
$image = $this->file('image');
$ext = $image->getClientOriginalExtension();
$file_name = $type.'-'.str_random(18).'.'.$ext;
\Log::debug('File name will be: '.$file_name);
if ($image->getClientOriginalExtension()!=='svg') {
\Log::debug('Not an SVG - resize');
\Log::debug('Trying to upload to: '.$path.'/'.$file_name);
$upload = Image::make($image->getRealPath())->resize(null, $w, function ($constraint) {
$constraint->aspectRatio();
$constraint->upsize();
})->save($path.'/'.$file_name);
} else {
\Log::debug('This is an SVG');
$sanitizer = new Sanitizer();
$dirtySVG = file_get_contents($image->getRealPath());
$cleanSVG = $sanitizer->sanitize($dirtySVG);
try {
\Log::debug('Trying to upload to: '.$path.'/'.$file_name);
file_put_contents($path.'/'.$file_name, $cleanSVG);
} catch (\Exception $e) {
\Log::debug($e);
}
}
// Remove Current image if exists
if (($item->image) && (file_exists($path.'/'.$item->image))) {
try {
unlink($path.'/'.$item->image);
} catch (\Exception $e) {
\Log::debug($e);
}
}
$item->image = $file_name;
}
} elseif ($this->input('image_delete')=='1') {
try {
unlink($path.'/'.$item->image);
} catch (\Exception $e) {
\Log::debug($e);
}
$item->image = null;
}
return $item;
}
}

1
composer.json
View file

@ -14,6 +14,7 @@
"doctrine/inflector": "^1.3",
"doctrine/instantiator": "^1.2",
"eduardokum/laravel-mail-auto-embed": "^1.0",
"enshrined/svg-sanitize": "^0.13.0",
"erusev/parsedown": "^1.7",
"fideloper/proxy": "^4.1",
"guzzlehttp/guzzle": "^6.3",

43
composer.lock generated
View file

@ -4,7 +4,7 @@
"Read more about it at https://getcomposer.org/doc/01-basic-usage.md#composer-lock-the-lock-file",
"This file is @generated automatically"
],
"content-hash": "cb4aca6c49fd1fd08aef1832b3e6762e",
"content-hash": "745e56814dad4b004d4d815075801416",
"packages": [
{
"name": "asm89/stack-cors",
@ -1286,6 +1286,47 @@
],
"time": "2019-08-13T17:33:27+00:00"
},
{
"name": "enshrined/svg-sanitize",
"version": "0.13.0",
"source": {
"type": "git",
"url": "https://github.com/darylldoyle/svg-sanitizer.git",
"reference": "4cf8d0f61edf9f00b84e162fc229176a362da247"
},
"dist": {
"type": "zip",
"url": "https://api.github.com/repos/darylldoyle/svg-sanitizer/zipball/4cf8d0f61edf9f00b84e162fc229176a362da247",
"reference": "4cf8d0f61edf9f00b84e162fc229176a362da247",
"shasum": ""
},
"require": {
"ext-dom": "*",
"ext-libxml": "*"
},
"require-dev": {
"codeclimate/php-test-reporter": "^0.1.2",
"phpunit/phpunit": "^6"
},
"type": "library",
"autoload": {
"psr-4": {
"enshrined\\svgSanitize\\": "src"
}
},
"notification-url": "https://packagist.org/downloads/",
"license": [
"GPL-2.0-or-later"
],
"authors": [
{
"name": "Daryll Doyle",
"email": "daryll@enshrined.co.uk"
}
],
"description": "An SVG sanitizer for PHP",
"time": "2019-11-07T09:16:31+00:00"
},
{
"name": "erusev/parsedown",
"version": "1.7.3",