Uploaded image for project: 'phpBB'
  1. phpBB
  2. PHPBB-9764

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

XMLWordPrintable

    • Icon: Bug Bug
    • Resolution: Fixed
    • Icon: Minor Minor
    • 3.0.9-RC1
    • 3.0.7-PL1
    • None
    • None
    • PHP 5.3.1, MySQL 5.1.41, Opera 10.60 / Firefox 3.0.5

      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.

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

              Created:
              Updated:
              Resolved: