Vulnerability - Able to upload script to server from VirtueMart

Started by anantmaks, June 01, 2018, 09:21:55 AM

Previous topic - Next topic

anantmaks

VirtueMart V3.2.14

This has been an old query and might be answered many times. Here I am highlighting something that can be an issue (if considered). VirtueMart in its image upload option allows any file to be uploaded (js, HTML, etc.), the thing I am able to get is that it also allows script and code to be sell if a vendor wants to. Though the thing is, even if it is so then those upload options(js, HTML, etc.) should be available on particular selections only. A user who wants to use the store as simple image upload option should not be able to upload those files that are executable.

If someone is still having doubt in what I am talking about, please refer to screenshots. I have created an HTML file->wrote some script in it->uploaded it in a category and product images->then open the link directly. My script executed uploaded on that server.
1. https://prnt.sc/jpdmcd
2. https://prnt.sc/jpdms7
3. https://prnt.sc/jpdqey

Now my questions are -
1. how safe it is?
2. Shouldn't we avoid it at least for some users, as we do restrict script addition by ACL settings?
3. Is it a good practice of impression for a software that it allows upload of other files than images, under its 'images' tab?

I started a topic on the non-functioning of ACL settings earlier which after a discussion been put in mods by Milbo. This concern is similar to that as many third-party developers build add-ons for VirtueMart and provide its back-end access on their demo. Now the users can add anything in this way onto developers server, which is actually not safe.
Anant Garg
Ghaziabad, India

Milbo

There are 2 different ACL options which controll this. As superuser you are allowed todo this, yes. But you can activate the filter of joomla and vm filter, which is less strict. So just check the ACL settings.
Should I fix your bug, please support the VirtueMart project and become a member
______________________________________
Extensions approved by the core team: http://extensions.virtuemart.net/

anantmaks

Hi Milbo

ok, I again tried it with a user with 'manager' level permissions. You were right ACL settings are working, though I found some glitches here and there which I would like to mention. You can check if they need improvement in future updates of VirtueMart.

For the manager, following permissions are set:
https://prnt.sc/jsdmdr
Now I tried uploading some different files:
1) .html => file doesn't upload and no entry is shown created
2) .php => file doesn't uploads though an entry is shown created **shouldn't it have worked the same way as .html file?**
3) .json => file uploads (same is the case with .js file) **Must not be an issue as JS can also be executed with tools in browser as well, though is it ok as per flow?**



Anant Garg
Ghaziabad, India

Studio 42

anantmaks, you are right.
But most of Joomla admin do not filter the shopowner rights.
But for multivendor shop, this is a real problem, because you cannot give right to upload images (and any other files) without adding security holes in current Virtuemart release.
Note that i check current ACL used about media
if(!vmAccess::manager('media.edit')){
vmWarn('Insufficient permission to store media');
return false;
} else if( empty($data['virtuemart_media_id']) and !vmAccess::manager('media.create')){
vmWarn('Insufficient permission to create media');
return false;
}

So no filter is set here.
I setting per joomla group should be used with a file filter and for media types because downloadable and product, category ... images do not need same restrictions.
But about product, category ... images, why not directly excluding any invalid image ?

Milbo

check the class vmUploader

There is


if(function_exists('exif_imagetype')){
$type = exif_imagetype($media['tmp_name']);
} else {
$type = false;
}

if($type){
vmdebug('Recognised image');
if(!self::checkMediaType($type,$mediaExtension)){
vmError('Invalid media, image type does not fit to extension '.$media['name'].' '.$type.'!='.$mediaExtension);
return false;
}
} else if(!vmAccess::manager('media.potdang')){

$m2ext = self::getMime2ExtArray();
$realMime = self::getMimeType($media['tmp_name']);

vmdebug('Uploading file $realMime',$realMime,$m2ext);
if(isset($m2ext[$realMime])){
//if($rExt = array_search($realMime,$m2ext)!==false){
$rExt = $m2ext[$realMime];
$hless = self::getSafeExt2MimeArray();
vmdebug('Recognised nonimage, not safe ext',$rExt,$hless);
//$rExt = $hless[$realMime];
if(!isset($hless[$rExt])){
vmError('Invalid media type, you are not allowed to upload this file, file type does not fit to mime '.$media['name']);
return false;
} else {
vmdebug('Uploading file ',$hless[$rExt]);
}
} else {
return false;
}
}

if($obj->file_is_forSale==0){

$uploadPath = VMPATH_ROOT.DS.$path_folder.$media['name'];
} else {
$uploadPath = $path_folder.$media['name'];
}
JFile::upload($media['tmp_name'], $uploadPath, false, vmAccess::manager('media.trusteduploader'));


So when the server supports exif_imagetype and the type is an image and the type fits, there is no further check, except the one of joomla (last parameter of the last line!), else we do a mime check by file extension.

The glitches are maybe due missing errorhandling, when joomla blocks the upload of a file

JFile::upload($media['tmp_name'], $uploadPath, false, vmAccess::manager('media.trusteduploader'));

$obj->file_mimetype = $media['type'];
$obj->media_published = 1;


We may need something like

$uploaded = JFile::upload($media['tmp_name'], $uploadPath, false, vmAccess::manager('media.trusteduploader'));
if(!$uploaded){
  return false;
}
$obj->file_mimetype = $media['type'];
$obj->media_published = 1;

Should I fix your bug, please support the VirtueMart project and become a member
______________________________________
Extensions approved by the core team: http://extensions.virtuemart.net/

Studio 42

exif_imagetype is not valid in all server.
I already have problem when you optimize tool with PNG images, because the header has no informations for exif and thumb is regenerate always.

Milbo

Quote from: Studio 42 on June 11, 2018, 13:27:05 PM
exif_imagetype is not valid in all server.
The reason we run another test with php, when exif is not supported.

Quote from: Studio 42 on June 11, 2018, 13:27:05 PM
I already have problem when you optimize tool with PNG images, because the header has no informations for exif and thumb is regenerate always.

This is fixed. Please try the new package on dev.virtuemart.net (uploaded today), or check the svn.
Should I fix your bug, please support the VirtueMart project and become a member
______________________________________
Extensions approved by the core team: http://extensions.virtuemart.net/

anantmaks

I am not sure if this should be a solution but please check. Can not we maintain a general rule that when an image (means a perfect image png, jpg, jpeg, etc) is uploaded, it goes into media directory and whenever any other file is uploaded, it goes into 'vmsafe' path? By this, the additional files will always only be downloadable and not executable (considering the 'vmsafe' path is a path not accessible by an URL).
This rule may remain same for all user irrespective of it is admin or any other, because a thing that is not safe should be avoided in practice for all.
Anant Garg
Ghaziabad, India