Securing renames

[olive31]

Hi there,

what would be the nicest way to modify the code so that folder names can be edited even in safe mode? I understand that the renaming of files has been disabled to avoid people uploading nasty code disguised in pictures and rename them afterward.

This is indeed a smart feature but renaming folder does not pose this sort of problem and is a feature interesting for uploaders to organise libraries.

The other option would also be to check that the renaming preserve the file extension. Afterall there should be no interest in changing a filetype ... Hence the safe mode feature would be only used to enable the change of filename extension.

As for my previous report, if you can point me roughly what to edit, I can try to make the change and send you the update

Best

[asika32764]

Hi,

About only renaming folder, the rename function is in elFinder, and it is a 3rd plugin. So I'm not sure how to implement this feature.

And I don't want to hack elFinder's code because it will meet problem when update to new version.

If you want to modify for your self, please check these files: https://github.com/asikart/remoteimage/blob/staging/admin/src/Elfinder/elFinderVolumeDriver.class.php https://github.com/asikart/remoteimage/blob/staging/admin/src/Elfinder/elFinderVolumeLocalFileSystem.class.php

elFinder config wiki: https://github.com/Studio-42/elFinder/wiki/Client-configuration-options

Hope this can help you.

Thank you.

[olive31]

Hi and thanks a lot for your prompt answer and your hints.

I analysed the code quickly and found that the security hole is in the elFinder code. There is no control when renaming a file to check that it still complies with the list of accepted/denied mimetypes.

Therefore, the easiest way I found to offer the user the possibility to rename files and folders without creating a security breach is to add this control in the elFinderVolumeDrive class, as follows.

Inserted at line 1326 in rename function: // Patch to check that the potentially new extension is accepted $deny = $this->mimeAccepted($this->mimetype($name), $this->uploadDeny, null); if ($deny) { return $this->setError(elFinder::ERROR_UPLOAD_FILE_MIME); } // End patch

I know this can't be a solution for you as this would pose problem when upgrading the version of elFinder.

The other possibility I see is to hook a nameValidator function (which is called when renaming) that would check that the filename is acceptable like I did in the code i above. This may be cleaner for you as this should be doable from your module without patching elFinder. However, I am not sufficiently familair with all this code to find quickly where to hook the function and what implications it can have.

Hope this can be useful to someone Best

PS: Do you define the list of accepted / denied mime types somewhere ?

[asika32764]

Hi,

I also not so familiar to elFinder, it old and some parts are hard to read. I wrote this extension last year and have been long time not to see my code.

About nameValidator, I see two parts in Driver class https://github.com/asikart/remoteimage/blob/staging/admin/src/Elfinder/elFinderVolumeDriver.class.php#L773 https://github.com/asikart/remoteimage/blob/staging/admin/src/Elfinder/elFinderVolumeDriver.class.php#L1992

I guess it can use by this way:

// src/Remoteimage/Controller/ManagerController.php

$opts['roots'][] = array(
    // ....
    'acceptedName' => 'someFunction' // Or regex string
);

$connector = new \elFinderConnector(new \elFinder($opts));
$connector->run();

// We need a raw function
function someFunction($fileName)
{
    // Do some stuff
    return (boolean) $result;
}

It weird that elFinder not accept a callback, class or object method, only raw function.

Hope helps.

PS: I only deny php file now, see: https://github.com/asikart/remoteimage/blob/staging/admin/src/Remoteimage/Controller/ManagerController.php#L58