Removing use of serialize() for locked fields. This removes any

possible exploit related to serialize()/unserialize(). Instead values are passed as | delimited.
cakephp · Nov 21, 2010 · 79aafda · 79aafda
1 parent ae78556
commit 79aafda
Show file tree

Hide file tree

Showing 2 changed files with 9 additions and 14 deletions.
diff --git a/cake/libs/view/helpers/form.php b/cake/libs/view/helpers/form.php
@@ -406,7 +406,7 @@ function secure($fields = array()) {
 		$fields += $locked;
 
 		$fields = Security::hash(serialize($fields) . Configure::read('Security.salt'));
-		$locked = str_rot13(serialize(array_keys($locked)));
+		$locked = implode(array_keys($locked), '|');
 
 		$out = $this->hidden('_Token.fields', array(
 			'value' => urlencode($fields . ':' . $locked),

diff --git a/cake/tests/cases/libs/view/helpers/form.test.php b/cake/tests/cases/libs/view/helpers/form.test.php
@@ -832,7 +832,7 @@ function testFormSecurityFields() {
 		$result = $this->Form->secure($fields);
 
 		$expected = Security::hash(serialize($fields) . Configure::read('Security.salt'));
-		$expected .= ':' . str_rot13(serialize(array('Model.valid')));
+		$expected .= ':' . 'Model.valid';
 
 		$expected = array(
 			'div' => array('style' => 'display:none;'),
@@ -894,9 +894,8 @@ function testFormSecurityMultipleFields() {
 		$this->Form->params['_Token']['key'] = $key;
 		$result = $this->Form->secure($fields);
 
-		$hash  = '51e3b55a6edd82020b3f29c9ae200e14bbeb7ee5%3An%3A4%3A%7Bv%3A0%3Bf%3A14%3A%22Zbqry.';
-		$hash .= '0.uvqqra%22%3Bv%3A1%3Bf%3A13%3A%22Zbqry.0.inyvq%22%3Bv%3A2%3Bf%3A14%3A%22Zbqry.1';
-		$hash .= '.uvqqra%22%3Bv%3A3%3Bf%3A13%3A%22Zbqry.1.inyvq%22%3B%7D';
+		$hash  = '51e3b55a6edd82020b3f29c9ae200e14bbeb7ee5%3AModel.0.hidden%7CModel.0.valid';
+		$hash .= '%7CModel.1.hidden%7CModel.1.valid';
 
 		$expected = array(
 			'div' => array('style' => 'display:none;'),
@@ -985,8 +984,7 @@ function testFormSecurityMultipleInputFields() {
 
 		$result = $this->Form->secure($this->Form->fields);
 
-		$hash = 'c9118120e680a7201b543f562e5301006ccfcbe2%3An%3A2%3A%7Bv%3A0%3Bf%3A14%';
-		$hash .= '3A%22Nqqerffrf.0.vq%22%3Bv%3A1%3Bf%3A14%3A%22Nqqerffrf.1.vq%22%3B%7D';
+		$hash = 'c9118120e680a7201b543f562e5301006ccfcbe2%3AAddresses.0.id%7CAddresses.1.id';
 
 		$expected = array(
 			'div' => array('style' => 'display:none;'),
@@ -1029,8 +1027,7 @@ function testFormSecurityMultipleInputDisabledFields() {
 		$this->Form->input('Addresses.1.phone');
 
 		$result = $this->Form->secure($this->Form->fields);
-		$hash = '774df31936dc850b7d8a5277dc0b890123788b09%3An%3A2%3A%7Bv%3A0%3Bf%3A14%3A%22Nqqerf';
-		$hash .= 'frf.0.vq%22%3Bv%3A1%3Bf%3A14%3A%22Nqqerffrf.1.vq%22%3B%7D';
+		$hash = '774df31936dc850b7d8a5277dc0b890123788b09%3AAddresses.0.id%7CAddresses.1.id';
 
 		$expected = array(
 			'div' => array('style' => 'display:none;'),
@@ -1074,8 +1071,7 @@ function testFormSecurityInputDisabledFields() {
 
 		$result = $this->Form->secure($expected);
 
-		$hash = '449b7e889128e8e52c5e81d19df68f5346571492%3An%3A1%3A%';
-		$hash .= '7Bv%3A0%3Bf%3A12%3A%22Nqqerffrf.vq%22%3B%7D';
+		$hash = '449b7e889128e8e52c5e81d19df68f5346571492%3AAddresses.id';
 		$expected = array(
 			'div' => array('style' => 'display:none;'),
 			'input' => array(
@@ -1179,8 +1175,7 @@ function testFormSecuredInput() {
 		);
 		$this->assertEqual($result, $expected);
 
-		$hash = 'bd7c4a654e5361f9a433a43f488ff9a1065d0aaf%3An%3A2%3A%7Bv%3A0%3Bf%3A15%3';
-		$hash .= 'A%22HfreSbez.uvqqra%22%3Bv%3A1%3Bf%3A14%3A%22HfreSbez.fghss%22%3B%7D';
+		$hash = 'bd7c4a654e5361f9a433a43f488ff9a1065d0aaf%3AUserForm.hidden%7CUserForm.stuff';
 
 		$result = $this->Form->secure($this->Form->fields);
 		$expected = array(
@@ -3569,7 +3564,7 @@ function testSelectMultipleCheckboxSecurity() {
 		$this->assertEqual($this->Form->fields, array('Model.multi_field'));
 
 		$result = $this->Form->secure($this->Form->fields);
-		$key = 'f7d573650a295b94e0938d32b323fde775e5f32b%3An%3A0%3A%7B%7D';
+		$key = 'f7d573650a295b94e0938d32b323fde775e5f32b%3A';
 		$this->assertPattern('/"' . $key . '"/', $result);
 	}