Fix for hardcoded uploads dir in User Avatar plugin

Super Interactive
(@superinteractive)

11 years, 7 months ago

This plugin wrongly uses a hardcoded reference to the wp-content and uploads dirs when creating the URL to display avatars.

The path and URL used for the wp-content dir should be dynamic, because it can be customized in wp-config.php. In this case the upload dir is referenced with the wp-content dir hardcoded in the URL.

Also, the plugin uses get_site_url() to create the URL for the upload dir. Apart from the fact that the upload dir can be found much easier (and correctly) using wp_upload_dir(), ‘siteurl’ and ‘home’ are two different things in WordPress; siteurl is where WordPress lives, home is the home URL of the website. These can be different when giving WordPress its own directory.

A patch to fix this can be found here:

https://gist.github.com/3705726

Hopefully you will be able to patch this soon, it is a great plugin!

http://wordpress.org/extend/plugins/user-avatar/

Viewing 5 replies - 1 through 5 (of 5 total)

Simon Blackbourn
(@lumpysimon)

11 years, 7 months ago

There are a couple more hardcoded URLs too, in user_avatar_core_avatar_upload_path() and user_avatar_core_avatar_url().

In user_avatar_delete() the site_url('/wp-admin') line should ideally use admin_url.

These minor points aside, it’s a really great plugin, easily the best one out there for allowing users to upload their own images.

Simon Blackbourn
(@lumpysimon)

11 years, 7 months ago

Just found another one on line 12 & 13 of timthumb-config.php.

Unfortunately because this file doesn’t have access to WordPress functions and constants it’s a bit trickier to deal with. I’ve added a ‘dir’ query variable to pass in the path where user-avatar-pic.php is called that gets round the problem, but I’m not sure if this might be a security problem or not.

dklawson
(@dklawson)

11 years, 6 months ago

Here’s a more complete patch:

https://gist.github.com/3899757

Lumpysimon – I would highly recommend reverting your ‘dir’ query variable patch. The patch would introduce a terrible vulnerability to your site. One of the first things timthumb does when it starts is to clean the cache. It sounds like you are allowing the cache directory to be configurable via your ‘dir’ query parm. Depending on how your file permissions are set up, someone could potentially cause your entire site (or worse) to be deleted by passing something like ‘dir=../../../’

I’m not sure what the intention is with putting the timthumb cache in the uploads folder, but as far as I can tell, it’s unnecessary. Taking out the FILE_CACHE_DIRECTORY define from timthumb-config.php will cause the default to be used (e.g. plugins/user-avatar/cache/) The files in the cache folder don’t even need to be publicly exposed since they are always processed by the timthumb code before they are returned anyway. This was part of the recent timthumb exploit. PHP code was being uploaded as “image” files and stored in the timthumb cache where it could be executed simply by visiting the URL. I’ve also taken an extra step by adding an .htaccess to the cache/ dir with “Deny From All” in it.

Ricardo Nunes
(@ricardopraia)

11 years, 6 months ago

dklawson I’m sorry for this stupid question but: Can you explain what do i have to do with your patch? What do i have to change?
Thanks

Simon Blackbourn
(@lumpysimon)

11 years, 6 months ago

@dklawson thanks for the heads-up & patch, i did think that wasn’t secure so i don’t have it running on any live sites.