Please upgrade here. These earlier versions are no longer being updated and have security issues.
HackerOne users: Testing against this community violates our program's Terms of Service and will result in your bounty being denied.

File Upload and Avatar Folder Names

Just setup my first Vanilla Forum coming from SMF/PHPBB mostly and coding my own sites in PHP, HTML5, etc. I mostly picked Vanilla for its size and Mobile support, and it allowed some features I wanted like invites. So far I've mostly cleaned house a bit- Set better Folder Permissions, turned off the error_reporting and display_errors in the index (Should never be set in production and opens up to hacking with returned Error strings), flipped the button order in Q and A along with making Discussion default over Question, set a max height on signatures, tightened FileUpload extensions and max file size, etc. I still need to find/fix some CSS with shifted backgrounds and checkboxes drawing on top of links, but everything else looks good.

When I created an Avatar that kept cropping and I changed it 3 times until I had a square under 50px that didn't crop. I noticed that the forum created a random folder and random file for each and kept the unused old folders and files. I also noticed the File Upload plugin appears to do the same thing. I haven't messed with it enough yet or looked too deep at the code to see if it does any house cleaning when posts containing an attachment are deleted, but I want to make it easier to clean up either way. My host has "Unlimited" files/bandwidth, but that really means 50,000 max can get backed up, so I don't want files everywhere.

My Idea is to have a folder for each user. This would most likely key off of the Users ID (Assuming they get an ID in the database...haven't looked at it yet). Then within that folder it could have random filenames except the Avatar which would be least to that extension type, so I'm not having new files every time someone changes their avatar. In class.fileupload.plugin it looks like it uses the first 2 of the MD5 for the folder and the rest for the file. What I need is the location where the Avatar is saved to edit it, and how to get the User's ID to use as the folder name. Might be useful to make it do this by default too in the future.


  • The code for this is confusing, but I think the Avatar upload is in class.profilecontroller.php.

    It calls GenerateTargetName to get a name like "/uploads/[Random 3 digit #]/[Random A-Z/0-9 12 char String].extension". GenerateTargetName is an odd function though supplying a default for $Extension then testing for it, and if $Chunk is False then the path returned would be "/uploads//[Random A-Z/0-9 12 char String].extension" which would be invalid.

    It then gets the Basename which is fine. To get the $Subdir though it calls StringBeginsWith with Trim set to true which appears to search for "/uploads/" within the dirname of the generated string of "/uploads/[Random 3 digit #]/[Random A-Z/0-9 12 char String].extension", but with a length of "/uploads/" it will always find that string. Since it is found and Trim was set it returns a Substring the length of "/uploads/"...which will be "/uploads/"???

    It then makes a string of all this with "userpics/$Subdir/p$Basename" which to me seems like it will be "userpics//uploads//p[Random A-Z/0-9 12 char String].extension". Seems like that is invalid.

    There is also a line in there of:

    // Delete any previously uploaded image.
    $UploadImage->Delete(ChangeBasename($this->User->Photo, 'p%s'));

    Which sounds to me like it should have been deleting my previous Avatars. Anyone know more on all this to explain it better, or am I in a totally wrong file?

  • edited October 2013

    Well, the File Upload was easy:

    // Build filename

    $SaveFilename = '/FileUpload/'.Gdn::Session()->UserID.'/'.md5(microtime()).'.'.strtolower($Extension);

    Appears the name propagates to thumbnail too, so all is done with one line and all user uploads go to the same folder. I also changed all the permissions in that file to 751 from 777. I did notice though that even before my changes when I deleted a picture the thumbnail wasn't deleted, so I need to find code for that now too in addition to figuring out how to do this for Avatars.

  • [Split]

    @Nilla_Wafer said:
    turned off the error_reporting and display_errors in the index (Should never be set in production and opens up to hacking with returned Error strings)

    Can you explain a bit more? I see a bug report coming out of this and we'll move the (split) discussion to Developers category for more technical discussion.

    If problem is still alive with 2.1 we definitely need a bug report on github

    There was an error rendering this rich post.

  • My index.php section looks like:

    // Report NO errors and track error_reporting(0); ini_set('display_errors',0); ini_set('track_errors', 1);

    I always turn off errors in PHP, otherwise when there is an error it can come as a text response in HTTP. Sometimes this can leak SQL strings, passwords, etc. When researching forums and their security I often look up hacks and exploits to see where to protect. Most of them for Vanilla crashed it in some way by requesting things out of range, etc. The HTTP response bodies all contained error text that gave information aiding in hacking the forum. Even with error reporting off, the major ones still log to an error_log file in the folder and the track_errors above seems to indicate a variable contains the error for debugging (assuming turning off the errors still populates it...I never used track_errors myself). I often get error_log files full of fatal exceptions and out of memory errors from people trying to hack things and gain info from the memory dumps and such in the error text that never comes to them since I have errors off.

  • Attempted to fix the issue of the thumbnail not removing in the trashfile method, but no luck (Used / and DS, but no difference):

    if (!$Deleted) { $DirectPath = MediaModel::PathUploads().DS.$Media->Path; if (file_exists($DirectPath)) $Deleted = @unlink($DirectPath); // Attempt to Delete Thumbnail $DirectPath = MediaModel::PathUploads() . "/thumbnails/" . $Media->Path; if (file_exists($DirectPath)) @unlink($DirectPath); }

Sign In or Register to comment.