Uploaded image for project: 'phpBB3'
  1. phpBB3
  2. PHPBB3-9764

Empty value for CONFIG_TABLE config_name= 'mime_triggers' causes functions_fileupload.php->fileupload->check_content() to be too restrictive

    XMLWordPrintable

Details

    • Bug
    • Status: Closed (View Workflow)
    • Minor
    • Resolution: Fixed
    • 3.0.7-PL1
    • 3.0.9-RC1
    • None
    • None
    • PHP 5.3.1, MySQL 5.1.41, Opera 10.60 / Firefox 3.0.5

    Description

      If

      SELECT config_value FROM *_config WHERE config_name= 'mime_triggers';

      gives an empty result or an empty string file uploads will be tested too restrictive for possible attack vectors. In /includes/functions_upload.php function check_content() will then search for a < only (with no additional text appended to it) in the 256 first bytes of the file, since we have an array of one element with an empty string! Of course this results in denying way too many files - < alone may appear very well in these files.

      This happens because in /includes/functions_user.php function avatar_upload($data, &$error) constructs fileupload blindly with the parameter explode('|', $config['mime_triggers']) - which of course gives a valid array having one element (with an empty string).

      To fix this I suggest to extend function set_disallowed_content() in /includes/functions_upload.php, which is now:

      	function set_disallowed_content($disallowed_content)
      	{
      		if ($disallowed_content !== false && is_array($disallowed_content))
      		{
      			$this->disallowed_content = $disallowed_content;
      		}
      	}

      ...to a more precise like:

      	function set_disallowed_content($disallowed_content)
      	{
      		if ($disallowed_content !== false && is_array($disallowed_content))
      		{
      			foreach ($disallowed_content as $k1 => $v1)  // Check each element
      			{
      				if ($v1== '')  // Element value is an empty string?
      				{
      					unset($disallowed_content[$k1]);  // Remove it from array
      				}
      			}
       
      			// Assigning an empty array is not a problem
      			$this->disallowed_content = $disallowed_content;
      		}
      	}

      This way empty array elements would be killed.

      Attachments

        Issue Links

          Activity

            People

              bantu Andreas Fischer [X] (Inactive)
              AmigoJack AmigoJack
              Votes:
              1 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved: