Howdy, Stranger!

It looks like you're new here. If you want to get involved, click one of these buttons!

Try Vanilla Forums Cloud product
Vanilla 2.6 is here! It includes security fixes and requires PHP 7.0. We have therefore ALSO released Vanilla 2.5.2 with security patches if you are still on PHP 5.6 to give you additional time to upgrade.

SFM Plugin: Enhancements and feedback

This discussion is related to the SFM addon.
jackmaessenjackmaessen ✭✭✭
edited April 2016 in Feedback

Enhancements and feedback are very welcome!

rbrahmson

Comments

  • hgtonighthgtonight ∞ · New Moderator

    This is a great idea. I love seeing new addons! :)

    I have a few suggestions:

    • Move the logic out of your view
    • Use the Gdn_Form class to write your form (simplifies parsing logic)
    • Use a JSON target to update your used percentage (removes the need for custom JS)
    • Use a model to track files rather than scanning the directory

    These changes will make your plugin even better than it already is. :chuffed:

    Search first

    Check out the Documentation! We are always looking for new content and pull requests.

    Click on insightful, awesome, and funny reactions to thank community volunteers for their valuable posts.

    jackmaessenBleistivtShadowdare
  • BleistivtBleistivt MVP
    edited April 2016

    In addition to that:

    • Don't allow uploading HTML files, this is an XSS vulnerability.
    • line 282 is a serious vulnerability, PM me if you need more information.
    • Use the UserID to identify users and their directories as the Username may change.
    • Save user directories in the uploads directory (use the constant PATH_UPLOADS), as other directories may not be writable.
    • Create a settings page for your plugin and use c() for config values.

    My themes: pure | minusbaseline - My plugins: CSSedit | HTMLedit | InfiniteScroll | BirthdayModule | [all] - PM me about customizations

    jackmaessenhgtonightShadowdare
  • @hgtonight and @Bleistivt thanks for the suggestions. I was not sure about the safety and the logic of the script so your comments are very welcome.

  • phreakphreak VanillaAPP - Your white label app for Vanillaforums MVP

    @jackmaessen: Wow, amazing! I have a wish or a feedback. My users always want to stay in control of their picture uploads via Advanced Editor (they are uploading photos of their babies and themselves, its part of their communication).

    Would it make sense to integrate these uploads there as well and make them for the users deleteable and maybe also viewable like a image gallery of all their uploaded files?

    MyAttachments was a pretty cool addon doing that. I guess its still working with FileUpload but i use the Advanced Editor in my boards.

    Willing to sponsor that integration into SFM.

  • vrijvlindervrijvlinder Papillon-Sauvage MVP

    MyAttachments was a pretty cool addon doing that. I guess its still working with FileUpload but i use the Advanced Editor in my boards.

    MyAttachments plugin works by grabbing any image uploaded by the user no matter how it was uploaded. Does it not work for you when you use Advanced Editor?

    If it no longer works I will have it deleted…

  • rbrahmsonrbrahmson "You may say I'm a dreamer / But I'm not the only one" NY ✭✭✭
    edited April 2016

    I haven't yet had the opportunity to try this plugin but the concept is great, thank you for this plugin! If the plugin doesn't do this already, here is a suggestion: when uploading zip files offer to expand it into the target folder.

    jackmaessen
  • i already fixed some issues from advised comments:

    • html and htm files not allowed anymore
    • delete action: fixed some vulnerability (thanks @Bleistivt )
    • used UserID instead of UserName
    • upload folder changed: uploads/sfm/md5 hash private folder

    Still need some help with the suggestions of @hgtonight:

    • Move the logic out of your view ( i have to move some php code to default.php ?)
    • Use the Gdn_Form class to write your form (simplifies parsing logic)
    • Use a JSON target to update your used percentage (removes the need for custom JS) really dont know how to handle with this
    • Use a model to track files rather than scanning the directory ( can you explain me this alternative for scanning dir)
    sfm.zip 20.9K
  • jackmaessenjackmaessen ✭✭✭
    edited May 2016

    Launched version 1.1 Now possible to create directories with subdirectories and upload your files in a folder or subfolder of your choice Total storage is recursively calculated
    Todo's for version 1.2
    Integrate a viewer for image files
    extract zip files in folder
    download folders; (creating .zip and download)

    Still need some more advice how to handle with @hgtonight suggestions:

    • Move the logic out of your view
    • Use the Gdn_Form class to write your form (simplifies parsing logic)
    • Use a JSON target to update your used percentage (removes the need for custom JS)
    • Use a model to track files rather than scanning the directory

    For those who are interested in the code of sfm.php; please read it critically and give me suggestions and improvements

  • hgtonighthgtonight ∞ · New Moderator

    shoot... I was working on the previous version.

    Basically, there should be no data manipulation in your view that doesn't directly relate to how it is output. For example, the list of files should be generated in your controller. You set the data to your controller, then you use that in the view.

    The upload form and handling should be done via the Gdn_Form and Gdn_Upload class.

    Here is a sample of the work in progress I had:

    <?php
    $PluginInfo['sfm'] = array(
        'Version' => '1.0',
        'Name' => 'SFM',
        'Description' => 'Simple File Management for users. Upload, View, Download and Delete files in your own folder.',
        'License'=> 'GNU GPL2',
        'Author' => 'Jack Maessen' 
    );
    
    class sfm_Plugin extends Gdn_Plugin {
        private $LimitStorage;
        private $MaxUploadSize;
        private $AllowedExts;
        private $ViewFiles;
        private $SumStorage;
        private $UsedStorage;
    
        public function __construct() {
            parent::__construct();
            $this->LimitStorage = c('SFM.StorageLimit');
            $this->MaxUploadSize = c('SFM.MaxUploasSize');
            $this->AllowedExts = c('SFM.AllowedExtensions');
            $this->ViewFiles = c('SFM.ViewFiles');
            $this->SumStorage = 0;
            $this->UsedStorage = 0;
        }
    
        public function Base_Render_Before($Sender) {
            $Sender->AddJsFile($this->GetResource('js/sfm.js', FALSE, FALSE));
            $Sender->AddCSSFile($this->GetResource('design/sfm.css', FALSE, FALSE));
            if ($Sender->Menu) {
                $Sender->Menu->AddLink('myfiles', T('MyFiles'), 'sfm');
            }
        }
    
        public function PluginController_sfm_Create($Sender) {
            if (!Gdn::Session()->IsValid() ) {
                return;
            }
    
            if ($Sender->Menu) {
                $Sender->ClearCssFiles();
                $Sender->AddCssFile('style.css');
                $Sender->MasterView = 'default';
            }
    
            $Sender->AddJsFile($this->GetResource('js/jquery.form.js', FALSE, FALSE));
            $Sender->AddCSSFile($this->GetResource('design/font-awesome.css', FALSE, FALSE));
    
            $this->dispatch($Sender);
        }
    
        public function controller_index($sender) {
            $UserDirectory = PATH_UPLOADS . '/sfm/' . md5(Gdn::session()->UserID);
    
            touchFolder($UserDirectory);
    
            foreach (new DirectoryIterator($UserDirectory) as $file) {
                if ($file->isFile()) {
                    $this->SumStorage += $file->getSize();
                }
            }
    
            $this->UsedStorage = $this->SumStorage / $this->LimitStorage * 100;    
    
            $sender->Render('sfm', '', 'plugins/sfm');
        }
    
    
        public function OnDisable() {
            $matchroute = 'sfm';
            Gdn::Router()-> DeleteRoute($matchroute); 
        }
    
        public function Setup() {
            touchConfig('SFM.StorageLimit', 20971520);
            touchConfig('SFM.MaxUploasSize', 2097152);
            touchConfig('SFM.AllowedExtensions', array('gif', 'jpeg', 'jpg', 'png', 'tif', 'tiff', 'zip', 'rar', 'js', 'css', 'less', 'txt', 'pdf', 'mp3'));
            touchConfig('SFM.ViewFiles', array('jpg', 'png', 'gif', 'tif', 'tiff', 'pdf'));
    
            $matchroute = 'sfm';
            $target = 'plugin/sfm';
            if(!Gdn::Router()->MatchRoute($matchroute)) {
                Gdn::Router()->SetRoute($matchroute,$target,'Internal');
            }
        }
    }
    

    Search first

    Check out the Documentation! We are always looking for new content and pull requests.

    Click on insightful, awesome, and funny reactions to thank community volunteers for their valuable posts.

    jackmaessenBleistivt
  • askeeaskee Россия New

    xm
    Fatal error: Call to undefined function mime_content_type() in Z:\home\wiki.ru\www\plugins\sfm\views\sfm.php on line 464
    File Type Time Size View Down Del Rename

  • R_JR_J Cheerleader & Troubleshooter Munich Moderator
  • jamesincjamesinc Sydney ✭✭

    I have been trying this out. One thing that struck me is that you're going to the effort of hashing the user upload directory, but in such a way as to make the hashes totally predictable. I amended my code to introduce a salt variable to make the upload directory names difficult to predict.

    default.php

    // Pinched from StackOverflow
    // https://stackoverflow.com/questions/2235434/how-to-generate-a-random-long-salt-for-use-in-hashing
    function rand_string( $length ) {
      $chars = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789";  
      $size = strlen( $chars );
      for( $i = 0; $i < $length; $i++ ) {
        $str .= $chars[ rand( 0, $size - 1 ) ];
      }
      return $str;
    }
    
    public function Setup() {
    
      $matchroute = 'sfm';
      $target = 'plugin/sfm';
    
      if(!Gdn::Router()->MatchRoute($matchroute))
        Gdn::Router()->SetRoute($matchroute,$target,'Internal'); 
    
      // Save salt value to Vanilla's config.php
      // if a salt is not already present
      if ( C('Plugins.sfm.Salt', '') == '' ) {
        // Generate a salt string
        $salt = rand_string(16);
    
        SaveToConfig('Plugins.sfm.Salt', $salt);
      }
    }
    

    sfm.php (line 15)

    $UserID = md5(Gdn::session() ->UserID . C('Plugins.sfm.Salt', 'SaltyMcSaltface'));
    
    hgtonightjackmaessen
  • jackmaessenjackmaessen ✭✭✭
    edited June 2016

    Good solution @jamesinc ! Really appreciated

  • Hello, i installed your addon thanks! there is a problem, i have the vanilla running under "mydomain/forum/" and when i click on MyFiles it redirect me on "mydomain/sfm" not to "mydomain/forum/sfm" doing so is not working. Any help please? thank you.

  • jackmaessenjackmaessen ✭✭✭
    edited July 2016

    If you have installed vanilla under the mydomain/forum/ subdomain, everything should work fine.
    So did you install Vanilla under the mydomain/forum/ folder?

  • @jackmaessen said:
    If you have installed vanilla under the mydomain/forum/ subdomain, everything should work fine.
    So did you install Vanilla under the mydomain/forum/ folder?

    Yes it is, but the MyFiles link doesnt work, here my website: www.proclanservers.com/forum/

    Thanks for your reply.

  • jackmaessenjackmaessen ✭✭✭
    edited July 2016

    Your url looks like this: http://www.proclanservers.com/forum/index.php?p=/sfm
    So this seems to be a problem with modification rewrite.
    Check if you have these lines in your .htaccess file

    # Original
    # If you modify this file then change the above line to: # Modified
    <IfModule mod_rewrite.c>
       RewriteEngine On
       # Certain hosts may require the following line.
       # If vanilla is in a subfolder then you need to specify it after the /.
       # (ex. You put Vanilla in /forum so change the next line to: RewriteBase /forum)
       # RewriteBase /
       RewriteCond %{REQUEST_FILENAME} !-d
       RewriteCond %{REQUEST_FILENAME} !-f
       RewriteRule ^(.*)$ index.php\?p=$1 [QSA,L]
    </IfModule>
    
  • TeoTeo New
    edited July 2016

    Hi thanks for your reply, my file is:
    # Original
    # If you modify this file then change the above line to: # Modified

    RewriteEngine On
    # Certain hosts may require the following line.
    # If vanilla is in a subfolder then you need to specify it after the /.
    # (ex. You put Vanilla in /forum so change the next line to: RewriteBase /forum)
    RewriteBase /forum
    RewriteCond %{REQUEST_FILENAME} !-d
    RewriteCond %{REQUEST_FILENAME} !-f
    RewriteRule ^(.*)$ index.php\?p=$1 [QSA,L]

    what's wrong please? thanks again.

  • jackmaessenjackmaessen ✭✭✭
    edited July 2016

    Well, it looks like he "redirects and dies" because of the url .
    Open file sfm.php in plugins/sfm/views/sfm.php and look for these lines:

    // we make an exception when url = /sfm and when in the url is a delete, compress or extract string,  which contains at least: delete=uploads/sfm/'.$UserID
         elseif ($_SERVER['QUERY_STRING'] != 'p=sfm'
             and strpos($_SERVER['QUERY_STRING'], 'delete=uploads/sfm/'.$UserID) != true 
             and strpos($_SERVER['QUERY_STRING'], 'compress=uploads/sfm/'.$UserID) != true
             and strpos($_SERVER['QUERY_STRING'], 'extract=uploads/sfm/'.$UserID) != true ) {
    
             //echo 'no access';
             $location = '/sfm';
             header("Location: " . "http://" . $_SERVER['HTTP_HOST'] . $location);
             die();
    
         }
    

    Now comment out the last 2 lines of it; so now it looks like this:

    // we make an exception when url = /sfm and when in the url is a delete, compress or extract string,  which contains at least: delete=uploads/sfm/'.$UserID
         elseif ($_SERVER['QUERY_STRING'] != 'p=sfm'
             and strpos($_SERVER['QUERY_STRING'], 'delete=uploads/sfm/'.$UserID) != true 
             and strpos($_SERVER['QUERY_STRING'], 'compress=uploads/sfm/'.$UserID) != true
             and strpos($_SERVER['QUERY_STRING'], 'extract=uploads/sfm/'.$UserID) != true ) {
    
             //echo 'no access';
             $location = '/sfm';
             //header("Location: " . "http://" . $_SERVER['HTTP_HOST'] . $location); // comment out
             //die(); // comment out
    
         }
    

    What happens now?

    Teo
  • TeoTeo New
    edited July 2016

    it was:

        // we make an exception when url = /sfm and when in the url is a delete, compress or extract string,  which contains at least: delete=uploads/sfm/'.$UserID
         elseif ($_SERVER['QUERY_STRING'] != 'p=sfm'
             and strpos($_SERVER['QUERY_STRING'], 'delete=uploads/sfm/'.$UserID) != true 
             and strpos($_SERVER['QUERY_STRING'], 'compress=uploads/sfm/'.$UserID) != true
             and strpos($_SERVER['QUERY_STRING'], 'extract=uploads/sfm/'.$UserID) != true ) {
    
             //echo 'no access';
             $location = '/sfm';
             header("Location: " . "http://" . $_SERVER['HTTP_HOST'] . $location);
             die();
    
         } 
    
    /* END PROTECT URL */
    

    now is as you said and it works!
    Thanks a lot XD

    Is there a way to have some kind of Downloads area with permission? Something where other can download files?

  • jackmaessenjackmaessen ✭✭✭
    edited July 2016

    @Teo
    This might be working now but you have to fix the vulnerability which has come up now.
    So put this line below in your script ( at the very end of it, after the last <?php in sfm.php) and look what output it creates:
    echo $_SERVER['QUERY_STRING'];
    Now you must copy the output into this line below:
    elseif ($_SERVER['QUERY_STRING'] != 'copy the output here'
    and you have to modify line 8 ( line 8 previous post)
    $location = '/index.php?p=/sfm';

    So it should now look like this:

    // we make an exception when url = /sfm and when in the url is a delete, compress or extract string,  which contains at least: delete=uploads/sfm/'.$UserID
         elseif ($_SERVER['QUERY_STRING'] != 'copy the output here'
             and strpos($_SERVER['QUERY_STRING'], 'delete=uploads/sfm/'.$UserID) != true 
             and strpos($_SERVER['QUERY_STRING'], 'compress=uploads/sfm/'.$UserID) != true
             and strpos($_SERVER['QUERY_STRING'], 'extract=uploads/sfm/'.$UserID) != true ) {
             //echo 'no access';
             $location = '/index.php?p=/sfm';
             header("Location: " . "http://" . $_SERVER['HTTP_HOST'] . $location);
             die();
         } 
    /* END PROTECT URL */
    

    Is there a way to have some kind of Downloads area with permission? Something where other can download files?

    Unfortunately: No not in this script....

  • Hi jackmaessen, thnaks for the heads up m8, here what i replaced it seems working fine:

        // we make an exception when url = /sfm and when in the url is a delete, compress or extract string,  which contains at least: delete=uploads/sfm/'.$UserID
             elseif ($_SERVER['QUERY_STRING'] != 'p=/sfm'
                 and strpos($_SERVER['QUERY_STRING'], 'delete=uploads/sfm/'.$UserID) != true 
                 and strpos($_SERVER['QUERY_STRING'], 'compress=uploads/sfm/'.$UserID) != true
                 and strpos($_SERVER['QUERY_STRING'], 'extract=uploads/sfm/'.$UserID) != true ) {
                 //echo 'no access';
                 $location = '/index.php?p=/sfm';
                 header("Location: " . "http://" . $_SERVER['HTTP_HOST'] . $location);
                 die();
             } 
        /* END PROTECT URL */
    
    jackmaessenPackham
Sign In or Register to comment.