Heine

  • home
  • drupal
  • about
Home

Unserializing user-supplied data, a bad idea

Heine — Wed, 25/08/2010 - 19:59

Apart from PHP bugs and Denial of Service attacks, there's another reason why calling unserialize on user-supplied data (cookies, hidden form fields) is a bad idea.

When a serialized object is unserialized, its __wakeup() member function will be called if it exists. Lesser known is the fact that another magic method will also be invoked. This is the __destruct() member function that will be invoked when either 1) all references to the object are removed, 2) when the object is explicitly destroyed or 3) when the script ends.

You can see for yourself with the following example code:

class someClass {
  public $foo;

  public function __destruct() {
    echo "__destruct(): Exterminate! Exterminate!\n";
  }
}

demo();
echo "Back in main.\n";

function demo() {
  echo "Demo starts.\n";
  $s = serialize(new SomeClass());
  var_dump($s);

  $u = unserialize($s);
  var_dump($u);
  echo "Demo's over.\n";
}

This will output:

Demo starts.
__destruct(): Exterminate! Exterminate!
string(32) "O:9:"someClass":1:{s:3:"foo";N;}"
object(someClass)#1 (1) {
  ["foo"]=>
  NULL
}
Demo's over.
__destruct(): Exterminate! Exterminate!
Back in main.

The first destruct happens when the result of new SomeClass() is destroyed, the second one when the $u variable goes out of scope and the last reference to the object is removed.

As all the member variables can be controlled by the person supplying the string, he (m/f/o) can cause a __destruct() member function to operate on unexpected data. Stefan Esser has used this to great effect; See Shocking news in PHP exploitation and the Piwik cookie unserialize vulnerability for how he was able to exploit this to run arbitrary code.

Drupal 7 also contains a __destruct() that, combined with the autoloader, can be used to delete arbitrary files from the system, provided the server running PHP has sufficient privileges to do so.

The __destruct() in question is a member function of Archive_Tar:

function __destruct()
{
  $this->_close();
  //...
}

function _close() {
  //...  
  if ($this->_temp_tarname != '') {
    @drupal_unlink($this->_temp_tarname);
    //...
  }
  //...
}

So, if a module uses unserialize on user-supplied data, one can simply provide the following string (adapting _temp_tarname) to do damage:

O:11:"Archive_Tar":6:{s:8:"_tarname";N;s:9:"_compress";b:0;s:14:"_compress_type";s:4:"none";s:10:"_separator";s:1:" ";s:5:"_file";i:0;s:13:"_temp_tarname";s:0:"";}

Take away message: do not call unserialize on user-supplied data.

Average: 4.5 (8 votes)
  • Planet Drupal
  • Security
  • Login to post comments

Does this apply to serialized arrays?

Anonymous (not verified) — Wed, 25/08/2010 - 23:00

Your analysis covers serialized objects but are there any security issues with using serialized arrays that contain user data?

  • Login to post comments

It is fine to unserialize

Heine — Thu, 26/08/2010 - 06:04

It is fine to unserialize something that you've serialized yourself earlier, whether objects or arrays, and where the user has no control over the string you pass to unserialize.

// Do

$foo = serialize ($some_user_supplied_date);
// stuff foo in db.

// later
// get info from db.
$restored_foo = unserialize($from_db);

What you should not do *) is call unserialize on a string provided by the user ($_POST, $_COOKIE etc). In that case, a string you'd expect to contain a seriallized array might very well contain an object. After all, the user can simply change the string.

// Don't
unserialize($_POST['hidden_form_field']);

* Some projects attempt to keep unserializing user-supplied data by adding a tamper warning; usually a hash, "signed" with a secret. While this may solve the problem, it comes with responsibilities, as one must

  • Protect against cryptographic splicing
  • Use a HMAC, not something vulnerable to extension attacks
  • Be resilient against timing attacks
  • Login to post comments

Good catch

Michael Prasuhn (not verified) — Thu, 26/08/2010 - 07:20

Wow, glad I never released this code. Are there significant reasons why someone would chose to post data that's been serialized vs. JSON encoded?

  • Login to post comments

I expect serialize is simply

Heine — Tue, 31/08/2010 - 09:39

I expect serialize is simply the first thing that comes to mind. After all, you'll get a full object back, instead of the stdClass json_decode gives you.

  • Login to post comments

Recent posts

  • In doubt? Read the specs!
  • About the Webform SA
  • Psalmen voor Altblokfluit
  • Unserializing user-supplied data, a bad idea
  • Planet Drupal past and current
more

Security reviews

  • Afraid custom code makes your site vulnerable?
  • You don't really trust that module you just downloaded from Drupal.org?

Sleep better after a security review.

Tags

bladmuziek blood donation blood supply CSRF Drupal Input Format modx MSM Performance Planet Drupal Security Varnish
more tags
  • home
  • drupal
  • about

Copyright © 2011 by Heine Deelstra. All rights reserved.