Permalink
Browse files

Fixing issue found by Felix Wilhelm(flxm) where users could send pote…

…ntially dangerous or corrupted serialized objects to SecurityComponent, potentially allowing manipulation of file map caches. Test case added.
  • Loading branch information...
1 parent 05572f6 commit f5714a821d0634a0905ae9bdd9dff3d15a8481cb @markstory markstory committed Nov 8, 2010
@@ -564,10 +564,15 @@ function _validatePost(&$controller) {
}
unset($check['_Token']);
+ $locked = str_rot13($locked);
+ if (preg_match('/(\A|;|{|})O\:[0-9]+/', $locked)) {
+ return false;
+ }
+
$lockedFields = array();
$fields = Set::flatten($check);
$fieldList = array_keys($fields);
- $locked = unserialize(str_rot13($locked));
+ $locked = unserialize($locked);
$multi = array();
foreach ($fieldList as $i => $key) {
@@ -551,6 +551,30 @@ function testValidatePostFormHacking() {
$result = $this->Controller->Security->validatePost($this->Controller);
$this->assertFalse($result, 'validatePost passed when key was missing. %s');
}
+
+/**
+ * Test that objects can't be passed into the serialized string. This was a vector for RFI and LFI
+ * attacks. Thanks to Felix Wilhelm
+ *
+ * @return void
+ */
+ function testValidatePostObjectDeserialize() {
+ $this->Controller->Security->startup($this->Controller);
+ $key = $this->Controller->params['_Token']['key'];
+ $fields = 'a5475372b40f6e3ccbf9f8af191f20e1642fd877';
+
+ // a corrupted serialized object, so we can see if it ever gets to deserialize
+ $attack = 'O:3:"App":1:{s:5:"__map";a:1:{s:3:"foo";s:7:"Hacked!";s:1:"fail"}}';
+ $fields .= urlencode(':' . str_rot13($attack));
+
+ $this->Controller->data = array(
+ 'Model' => array('username' => 'mark', 'password' => 'foo', 'valid' => '0'),
+ '_Token' => compact('key', 'fields')
+ );
+ $result = $this->Controller->Security->validatePost($this->Controller);
+ $this->assertFalse($result, 'validatePost passed when key was missing. %s');
+ }
+
/**
* Tests validation of checkbox arrays
*

0 comments on commit f5714a8

Please sign in to comment.