Love Fuel?    Donate

FuelPHP Forums

Ask your question about FuelPHP in the appropriate forum, or help others by answering their questions.
Question on Security class
  • I was doing some testing to check the output encoding sent to a view and realized that most of the data was not being encoded. It turns out that I'm pulling data from the database as an object or array of objects. These objects are the PHP stdClass and by default these are whitelisted in the config.php file. I'm wondering what others are doing and what the best solution would be. I could pull the data from the database as arrays, but I would have to change a lot of code, and I like using the format $x->y as opposed to $x. I believe that it's not possible to set up a __toString() method for stdClass. Any other thoughts or suggestions would be appreciated.
  • Jelmer, I just changed the following lines in htmlentities() function as you suggested and tested it out. When I use an array of objects, I get the error: "Cannot use object of type stdClass as array" at the line: $value[$k] = static::htmlentities($v); Because of the recursive calling to htmlentities(), the error occurs when $value is a stdClass object.
      elseif (is_array($value) || $value instanceof \Iterator || get_class($value) == 'stdClass')
      {
       // Add to $already_cleaned variable when object
       is_object($value) and $already_cleaned[] = $value;
    
       foreach ($value as $k => $v)
       {
        $value[$k] = static::htmlentities($v);
       }
      }
    
  • One other thing, by putting the check for stdClass in the line:
    elseif (is_array($value) || $value instanceof \Iterator || get_class($value) == 'stdClass')
    
    it disallows whitelisting stdClass because the check for stdClass now comes before the check for security.whitelisted_classes.
  • I'm sure someone else can recode this in a more elegant way, but this seems to work:
     public static function htmlentities($value)
     {
      static $already_cleaned = array();
    
      // Nothing to escape for non-string scalars, or for already processed values
      if (is_bool($value) or is_int($value) or is_float($value) or in_array($value, $already_cleaned, true))
      {
       return $value;
      }
    
      if (is_string($value))
      {
       $value = htmlentities($value, ENT_COMPAT, \Fuel::$encoding, false);
      }
      elseif (is_array($value) || $value instanceof \Iterator)
      {
       // Add to $already_cleaned variable when object
       is_object($value) and $already_cleaned[] = $value;
    
       foreach ($value as $k => $v)
       {
        $value[$k] = static::htmlentities($v);
       }
      }
      elseif (is_object($value))
      {
       // Check if the object is whitelisted and return when that's the case
       foreach (\Config::get('security.whitelisted_classes') as $class)
       {
        if (is_a($value, $class))
        {
         // Add to $already_cleaned variable
         $already_cleaned[] = $value;
    
         return $value;
        }
       }
    
       // Throw exception when it wasn't whitelisted and can't be converted to String
       if (method_exists($value, '__toString'))
       {
        $value = static::htmlentities((string) $value);
       }
       else if (get_class($value) == 'stdClass')
       {
        foreach ($value as $k => $v)
        {
         if (is_array($value))
         {
          $stdclass = 0;
         }
         else if (get_class($value) == 'stdClass')
         {
          $stdclass = 1;
          $value = (array)$value;
         }
    
         $value[$k] = static::htmlentities($v);
    
         if ($stdclass)
         {
          $value = (object)$value;
         }
        }
       }
       else
       {
        throw new \RuntimeException('Object class "'.get_class($value).'" could not be converted to string or '.
         'sanitized as ArrayAcces. Whitelist it in security.whitelisted_classes in app/config/config.php '.
         'to allow it to be passed unchecked.');
       }
      }
    
      return $value;
     }
    
  • I don't know why it was decided to whitelist a stdClass, as it 's perfectly possible to process them (although it will not be fast). Maybe Jelmer can comment?
  • Just checked it out and it's a mistake, Phil added it a while back but it should never have been put there.
  • Even if I remove stdClass from the whitelist, because it's an object it won't be sanitized unless there's a __toString() method associated with it. I don't believe it's possible to associate a __toString method with stdClass.
  • Good point. I added stdClass as foreachable like Iterator objects & arrays as it can't have any hidden properties and should thus be fully sanitized when done foreaching.
  • Thanks Jelmer. Where can I find the file? I checked the core and issues pages but didn't see it.
  • It's in 1.1/develop branches of github.com/fuel/fuel & github.com/fuel/core - it's 2 changes: 1. Remove 'stdClass' from your array in app/config/config.php
    2. Added the last boolean condition on this line I may also update the 1.0 develop branches with this, but I'm not sure yet as it'll be a pretty big change for many I'm afraid - but it's also a security concern.

Howdy, Stranger!

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

In this Discussion