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

    Details

    • Type: Bug
    • Status: Unverified Fix
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 3.0.7-PL1
    • Fix Version/s: 3.0.9-RC1
    • Component/s: None
    • Labels:
      None
    • Environment:
      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

              • Assignee:
                bantu Andreas Fischer
                Reporter:
                AmigoJack AmigoJack
              • Votes:
                1 Vote for this issue
                Watchers:
                1 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: