diff --git a/lib/filter.php b/lib/filter.php index 623b8bb..2face50 100644 --- a/lib/filter.php +++ b/lib/filter.php @@ -50,4 +50,35 @@ class filter } return number_format($size, ($i ? 2 : 0), '.', ' ') . ' ' . $iec[$i]; } + + /** + * validate paste ID + * + * @access public + * @param string $dataid + * @return bool + */ + public static function is_valid_paste_id($dataid) + { + return (bool) preg_match('#\A[a-f\d]{16}\z#', $dataid); + } + + /** + * fixed time string comparison operation to prevent timing attacks + * https://crackstation.net/hashing-security.htm?=rd#slowequals + * + * @access public + * @param string $a + * @param string $b + * @return bool + */ + public static function slow_equals($a, $b) + { + $diff = strlen($a) ^ strlen($b); + for($i = 0; $i < strlen($a) && $i < strlen($b); $i++) + { + $diff |= ord($a[$i]) ^ ord($b[$i]); + } + return $diff === 0; + } } diff --git a/lib/zerobin.php b/lib/zerobin.php index 0fb350b..e5ca81c 100644 --- a/lib/zerobin.php +++ b/lib/zerobin.php @@ -272,8 +272,8 @@ class zerobin $pasteid = $_POST['pasteid']; $parentid = $_POST['parentid']; if ( - !preg_match('#\A[a-f\d]{16}\z#', $pasteid) || - !preg_match('#\A[a-f\d]{16}\z#', $parentid) + !filter::is_valid_paste_id($pasteid) || + !filter::is_valid_paste_id($parentid) ) $this->_return_message(1, 'Invalid data.'); // Comments do not expire (it's the paste that expires) @@ -340,18 +340,21 @@ class zerobin private function _delete($dataid, $deletetoken) { // Is this a valid paste identifier? - if (preg_match('#\A[a-f\d]{16}\z#', $dataid)) + if (!filter::is_valid_paste_id($dataid)) { - // Check that paste exists. - if (!$this->_model()->exists($dataid)) - { - $this->_error = 'Paste does not exist, has expired or has been deleted.'; - return; - } + $this->_error = 'Invalid paste ID.'; + return; + } + + // Check that paste exists. + if (!$this->_model()->exists($dataid)) + { + $this->_error = 'Paste does not exist, has expired or has been deleted.'; + return; } // Make sure token is valid. - if ($deletetoken != hash_hmac('sha1', $dataid , serversalt::get())) + if (filter::slow_equals($deletetoken, hash_hmac('sha1', $dataid , serversalt::get()))) { $this->_error = 'Wrong deletion token. Paste was not deleted.'; return; @@ -372,63 +375,62 @@ class zerobin private function _read($dataid) { // Is this a valid paste identifier? - if (preg_match('#\A[a-f\d]{16}\z#', $dataid)) + if (!filter::is_valid_paste_id($dataid)) { - // Check that paste exists. - if ($this->_model()->exists($dataid)) + $this->_error = 'Invalid paste ID.'; + return; + } + + // Check that paste exists. + if ($this->_model()->exists($dataid)) + { + // Get the paste itself. + $paste = $this->_model()->read($dataid); + + // See if paste has expired. + if ( + isset($paste->meta->expire_date) && + $paste->meta->expire_date < time() + ) + { + // Delete the paste + $this->_model()->delete($dataid); + $this->_error = 'Paste does not exist, has expired or has been deleted.'; + } + // If no error, return the paste. + else { - // Get the paste itself. - $paste = $this->_model()->read($dataid); + // We kindly provide the remaining time before expiration (in seconds) + if ( + property_exists($paste->meta, 'expire_date') + ) $paste->meta->remaining_time = $paste->meta->expire_date - time(); - // See if paste has expired. + // The paste itself is the first in the list of encrypted messages. + $messages = array($paste); + + // If it's a discussion, get all comments. if ( - isset($paste->meta->expire_date) && - $paste->meta->expire_date < time() + property_exists($paste->meta, 'opendiscussion') && + $paste->meta->opendiscussion ) { - // Delete the paste - $this->_model()->delete($dataid); - $this->_error = 'Paste does not exist, has expired or has been deleted.'; + $messages = array_merge( + $messages, + $this->_model()->readComments($dataid) + ); } - // If no error, return the paste. - else - { - // We kindly provide the remaining time before expiration (in seconds) - if ( - property_exists($paste->meta, 'expire_date') - ) $paste->meta->remaining_time = $paste->meta->expire_date - time(); - - // The paste itself is the first in the list of encrypted messages. - $messages = array($paste); - - // If it's a discussion, get all comments. - if ( - property_exists($paste->meta, 'opendiscussion') && - $paste->meta->opendiscussion - ) - { - $messages = array_merge( - $messages, - $this->_model()->readComments($dataid) - ); - } - $this->_data = json_encode($messages); - - // If the paste was meant to be read only once, delete it. - if ( - property_exists($paste->meta, 'burnafterreading') && - $paste->meta->burnafterreading - ) $this->_model()->delete($dataid); - } - } - else - { - $this->_error = 'Paste does not exist or has expired.'; + $this->_data = json_encode($messages); + + // If the paste was meant to be read only once, delete it. + if ( + property_exists($paste->meta, 'burnafterreading') && + $paste->meta->burnafterreading + ) $this->_model()->delete($dataid); } } else { - $this->_error = 'Invalid paste ID.'; + $this->_error = 'Paste does not exist or has expired.'; } } diff --git a/tst/filter.php b/tst/filter.php index c8d4239..c058a66 100644 --- a/tst/filter.php +++ b/tst/filter.php @@ -44,4 +44,25 @@ class filterTest extends PHPUnit_Framework_TestCase $this->assertEquals('1.00 YiB', filter::size_humanreadable(1024 * $exponent)); $this->assertEquals('1.21 YiB', filter::size_humanreadable(1234 * $exponent)); } + + public function testPasteIdValidation() + { + $this->assertTrue(filter::is_valid_paste_id('a242ab7bdfb2581a'), 'valid paste id'); + $this->assertFalse(filter::is_valid_paste_id('foo'), 'invalid hex values'); + $this->assertFalse(filter::is_valid_paste_id('../bar/baz'), 'path attack'); + } + + public function testSlowEquals() + { + $this->assertTrue(filter::slow_equals('foo', 'foo'), 'same string'); + $this->assertFalse(filter::slow_equals('foo', true), 'string and boolean'); + $this->assertFalse(filter::slow_equals('foo', 0), 'string and integer'); + $this->assertFalse(filter::slow_equals('123foo', 123), 'string and integer'); + $this->assertFalse(filter::slow_equals('123foo', '123'), 'different strings'); + $this->assertFalse(filter::slow_equals('6', ' 6'), 'strings with space'); + $this->assertFalse(filter::slow_equals('4.2', '4.20'), 'floats as strings'); + $this->assertFalse(filter::slow_equals('1e3', '1000'), 'integers as strings'); + $this->assertFalse(filter::slow_equals('9223372036854775807', '9223372036854775808'), 'large integers as strings'); + $this->assertFalse(filter::slow_equals('61529519452809720693702583126814', '61529519452809720000000000000000'), 'larger integers as strings'); + } } diff --git a/tst/mcrypt_mock.php b/tst/mcrypt_mock.php new file mode 100644 index 0000000..96b0ed3 --- /dev/null +++ b/tst/mcrypt_mock.php @@ -0,0 +1,17 @@ + ./ + mcrypt_mock.php diff --git a/tst/vizhash16x16.php b/tst/vizhash16x16.php index 82f06a0..b33d48d 100644 --- a/tst/vizhash16x16.php +++ b/tst/vizhash16x16.php @@ -37,5 +37,10 @@ class vizhash16x16Test extends PHPUnit_Framework_TestCase $this->assertEquals('image/png', $finfo->file($this->_file)); $this->assertNotEquals($pngdata, $vz->generate('2001:1620:2057:dead:beef::cafe:babe')); $this->assertEquals($pngdata, $vz->generate('127.0.0.1')); + + // generating new salt + $salt = serversalt::get(); + require 'mcrypt_mock.php'; + $this->assertNotEquals($salt, serversalt::generate()); } }