Uploaded image for project: 'CiviCRM'
  1. CiviCRM
  2. CRM-19208

guesURL function in Security.php can produce wrong outcome

    Details

    • Type: Bug
    • Status: Open
    • Priority: Minor
    • Resolution: Unresolved
    • Affects Version/s: 4.7.10
    • Fix Version/s: Unscheduled
    • Component/s: Core CiviCRM
    • Labels:
      None
    • Versioning Impact:
      Patch (backwards-compatible bug fixes)
    • Documentation Required?:
      None
    • Funding Source:
      Needs Funding

      Description

      When installing civicrm by default (on wordpress, but it is for all cms), the image upload url gets defined as
      [civicrm.files]/persist/contribute/
      Now, in CRM/Utils/Check/Component/Security.php, function guessUrl (line 370 and following), we have the following logic:

          $filePathMarker = $this->getFilePathMarker();
          $config = CRM_Core_Config::singleton();
      
          list ($heuristicBaseUrl, $ignore) = explode($filePathMarker, $config->imageUploadURL);
          list ($ignore, $heuristicSuffix) = explode($filePathMarker, str_replace('\\', '/', $targetDir));
      

      The variable $filePathMarker is defined higher up in that file and is either "/media/" (for joomla) or "/files/" (for other cms). Based on this, the first explode-call returns an array with only 1 element (and the second one too, but that probably depends on $targetDir), resulting in these php warnings:

      Notice: Undefined offset: 1 in /var/www/html/wordpress/wp-content/plugins/civicrm/civicrm/CRM/Utils/Check/Component/Security.php on line 374
      Notice: Undefined offset: 1 in /var/www/html/wordpress/wp-content/plugins/civicrm/civicrm/CRM/Utils/Check/Component/Security.php on line 375

      We can easily work around this by checking the array returned from explode to account for these warnings (I can create a PR for that if you want).
      The thing is: what even happens when someone changes the civicrm setting to be something like "[civicrm.files]/files/"? Or worse: /var/www/files/civicrm/upload/files/
      When checking the generate file civicrm.settings.php, I even see

      // Override the images directory.
      // $civicrm_setting['Directory Preferences']['imageUploadDir'] = '/path/to/image-upload-dir' ;

      Which seems weird since you can set this setting in the admin interface ...

      So I'm a bit confused here. I can already create a PR if you want but would like some feedback on the rest

        Attachments

          Activity

            People

            • Assignee:
              Unassigned
              Reporter:
              liedekef Franky Van Liedekerke
            • Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated: