From cc53d95ed1f470a8143e8a56aa3192e2c4b3e13e Mon Sep 17 00:00:00 2001 From: El RIDO Date: Sun, 20 Jan 2019 11:05:34 +0100 Subject: [PATCH 1/9] extending test cases to reproduce the issue from #396, causing the existing logic to now fail the tests --- js/common.js | 17 ++++++--- js/test/Model.js | 15 +++++--- tst/RequestTest.php | 86 ++++++++++++++++++++++++++++++++++++--------- 3 files changed, 92 insertions(+), 26 deletions(-) diff --git a/js/common.js b/js/common.js index bc3bd399..be1cf377 100644 --- a/js/common.js +++ b/js/common.js @@ -22,11 +22,13 @@ require('./bootstrap-3.3.7'); require('./privatebin'); // internal variables -var a2zString = ['a','b','c','d','e','f','g','h','i','j','k','l','m', - 'n','o','p','q','r','s','t','u','v','w','x','y','z'], - alnumString = a2zString.concat(['0','1','2','3','4','5','6','7','8','9']), - queryString = alnumString.concat(['+','%','&','.','*','-','_']), - hashString = queryString.concat(['!']), +var a2zString = ['a','b','c','d','e','f','g','h','i','j','k','l','m', + 'n','o','p','q','r','s','t','u','v','w','x','y','z'], + digitString = ['0','1','2','3','4','5','6','7','8','9'], + alnumString = a2zString.concat(digitString), + hexString = digitString.concat(['a','b','c','d','e','f']), + queryString = alnumString.concat(['+','%','&','.','*','-','_']), + hashString = queryString.concat(['!']), base64String = alnumString.concat(['+','/','=']).concat( a2zString.map(function(c) { return c.toUpperCase(); @@ -123,6 +125,11 @@ exports.jscAlnumString = function() { return jsc.elements(alnumString); }; +//provides random characters allowed in hexadecimal notation +exports.jscHexString = function() { + return jsc.elements(hexString); +}; + // provides random characters allowed in GET queries exports.jscQueryString = function() { return jsc.elements(queryString); diff --git a/js/test/Model.js b/js/test/Model.js index 18ec8a89..1ed88127 100644 --- a/js/test/Model.js +++ b/js/test/Model.js @@ -81,18 +81,23 @@ describe('Model', function () { 'returns the query string without separator, if any', jsc.nearray(common.jscA2zString()), jsc.nearray(common.jscA2zString()), - jsc.nearray(common.jscHashString()), + jsc.tuple(new Array(16).fill(common.jscHexString)), + jsc.array(common.jscQueryString()), + jsc.array(common.jscQueryString()), 'string', - function (schema, address, query, fragment) { - var queryString = query.join(''), - clean = jsdom('', { + function (schema, address, pasteId, queryStart, queryEnd, fragment) { + var pasteIdString = pasteId.join(''), + queryStartString = queryStart.join('') + (queryStart.length > 0 ? '&' : ''), + queryEndString = (queryEnd.length > 0 ? '&' : '') + queryEnd.join(''), + queryString = queryStartString + pasteIdString + queryEndString, + clean = jsdom('', { url: schema.join('') + '://' + address.join('') + '/?' + queryString + '#' + fragment }), result = $.PrivateBin.Model.getPasteId(); $.PrivateBin.Model.reset(); clean(); - return queryString === result; + return pasteIdString === result; } ); jsc.property( diff --git a/tst/RequestTest.php b/tst/RequestTest.php index 29b0dade..c13656db 100644 --- a/tst/RequestTest.php +++ b/tst/RequestTest.php @@ -21,6 +21,36 @@ class RequestTest extends PHPUnit_Framework_TestCase $_POST = array(); } + /** + * Returns 16 random hexadecimal characters. + * + * @access public + * @return string + */ + public function getRandomId() + { + // 8 binary bytes are 16 characters long in hex + return bin2hex(random_bytes(8)); + } + + /** + * Returns random query safe characters. + * + * @access public + * @return string + */ + public function getRandomQueryChars() + { + $queryChars = '0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ='; + $queryCharCount = strlen($queryChars) - 1; + $resultLength = random_int(1, 10); + $result = ''; + for ($i = 0; $i < $resultLength; ++$i) { + $result .= $queryChars[random_int(0, $queryCharCount)]; + } + return $result; + } + public function testView() { $this->reset(); @@ -33,24 +63,26 @@ class RequestTest extends PHPUnit_Framework_TestCase public function testRead() { $this->reset(); + $id = $this->getRandomId(); $_SERVER['REQUEST_METHOD'] = 'GET'; - $_SERVER['QUERY_STRING'] = 'foo'; + $_SERVER['QUERY_STRING'] = $id; $request = new Request; $this->assertFalse($request->isJsonApiCall(), 'is HTML call'); - $this->assertEquals('foo', $request->getParam('pasteid')); + $this->assertEquals($id, $request->getParam('pasteid')); $this->assertEquals('read', $request->getOperation()); } public function testDelete() { $this->reset(); + $id = $this->getRandomId(); $_SERVER['REQUEST_METHOD'] = 'GET'; - $_GET['pasteid'] = 'foo'; + $_GET['pasteid'] = $id; $_GET['deletetoken'] = 'bar'; $request = new Request; $this->assertFalse($request->isJsonApiCall(), 'is HTML call'); $this->assertEquals('delete', $request->getOperation()); - $this->assertEquals('foo', $request->getParam('pasteid')); + $this->assertEquals($id, $request->getParam('pasteid')); $this->assertEquals('bar', $request->getParam('deletetoken')); } @@ -84,74 +116,96 @@ class RequestTest extends PHPUnit_Framework_TestCase public function testApiRead() { $this->reset(); + $id = $this->getRandomId(); $_SERVER['REQUEST_METHOD'] = 'GET'; $_SERVER['HTTP_ACCEPT'] = 'application/json, text/javascript, */*; q=0.01'; - $_SERVER['QUERY_STRING'] = 'foo'; + $_SERVER['QUERY_STRING'] = $id; $request = new Request; $this->assertTrue($request->isJsonApiCall(), 'is JSON Api call'); - $this->assertEquals('foo', $request->getParam('pasteid')); + $this->assertEquals($id, $request->getParam('pasteid')); $this->assertEquals('read', $request->getOperation()); } public function testApiDelete() { $this->reset(); + $id = $this->getRandomId(); $_SERVER['REQUEST_METHOD'] = 'POST'; $_SERVER['HTTP_X_REQUESTED_WITH'] = 'JSONHttpRequest'; - $_SERVER['QUERY_STRING'] = 'foo'; + $_SERVER['QUERY_STRING'] = $id; $_POST['deletetoken'] = 'bar'; $request = new Request; $this->assertTrue($request->isJsonApiCall(), 'is JSON Api call'); $this->assertEquals('delete', $request->getOperation()); - $this->assertEquals('foo', $request->getParam('pasteid')); + $this->assertEquals($id, $request->getParam('pasteid')); $this->assertEquals('bar', $request->getParam('deletetoken')); } public function testReadWithNegotiation() { $this->reset(); + $id = $this->getRandomId(); $_SERVER['REQUEST_METHOD'] = 'GET'; $_SERVER['HTTP_ACCEPT'] = 'text/html,text/html; charset=UTF-8,application/xhtml+xml, application/xml;q=0.9,*/*;q=0.8, text/csv,application/json'; - $_SERVER['QUERY_STRING'] = 'foo'; + $_SERVER['QUERY_STRING'] = $id; $request = new Request; $this->assertFalse($request->isJsonApiCall(), 'is HTML call'); - $this->assertEquals('foo', $request->getParam('pasteid')); + $this->assertEquals($id, $request->getParam('pasteid')); $this->assertEquals('read', $request->getOperation()); } public function testReadWithXhtmlNegotiation() { $this->reset(); + $id = $this->getRandomId(); $_SERVER['REQUEST_METHOD'] = 'GET'; $_SERVER['HTTP_ACCEPT'] = 'application/xhtml+xml,text/html,text/html; charset=UTF-8, application/xml;q=0.9,*/*;q=0.8, text/csv,application/json'; - $_SERVER['QUERY_STRING'] = 'foo'; + $_SERVER['QUERY_STRING'] = $id; $request = new Request; $this->assertFalse($request->isJsonApiCall(), 'is HTML call'); - $this->assertEquals('foo', $request->getParam('pasteid')); + $this->assertEquals($id, $request->getParam('pasteid')); $this->assertEquals('read', $request->getOperation()); } public function testApiReadWithNegotiation() { $this->reset(); + $id = $this->getRandomId(); $_SERVER['REQUEST_METHOD'] = 'GET'; $_SERVER['HTTP_ACCEPT'] = 'text/plain,text/csv, application/xml;q=0.9, application/json, text/html,text/html; charset=UTF-8,application/xhtml+xml, */*;q=0.8'; - $_SERVER['QUERY_STRING'] = 'foo'; + $_SERVER['QUERY_STRING'] = $id; $request = new Request; $this->assertTrue($request->isJsonApiCall(), 'is JSON Api call'); - $this->assertEquals('foo', $request->getParam('pasteid')); + $this->assertEquals($id, $request->getParam('pasteid')); $this->assertEquals('read', $request->getOperation()); } public function testReadWithFailedNegotiation() { $this->reset(); + $id = $this->getRandomId(); $_SERVER['REQUEST_METHOD'] = 'GET'; $_SERVER['HTTP_ACCEPT'] = 'text/plain,text/csv, application/xml;q=0.9, */*;q=0.8'; - $_SERVER['QUERY_STRING'] = 'foo'; + $_SERVER['QUERY_STRING'] = $id; $request = new Request; $this->assertFalse($request->isJsonApiCall(), 'is HTML call'); - $this->assertEquals('foo', $request->getParam('pasteid')); + $this->assertEquals($id, $request->getParam('pasteid')); $this->assertEquals('read', $request->getOperation()); } + + public function testPasteIdExtraction() + { + $this->reset(); + $id = $this->getRandomId(); + $queryParams = array($id); + $queryParamCount = random_int(1, 5); + for ($i = 0; $i < $queryParamCount; ++$i) { + array_push($queryParams, $this->getRandomQueryChars()); + } + shuffle($queryParams); + $_SERVER['REQUEST_METHOD'] = 'GET'; + $_SERVER['QUERY_STRING'] = implode('&', $queryParams); + $request = new Request; + $this->assertEquals($id, $request->getParam('pasteid')); + } } From 79a858f176947db963a202fecdabb62a89979d82 Mon Sep 17 00:00:00 2001 From: El RIDO Date: Sun, 20 Jan 2019 12:20:37 +0100 Subject: [PATCH 2/9] extracting only the 16 hex characters of the query string as paste ID, addressing #396 --- js/privatebin.js | 2 +- js/test/Prompt.js | 2 +- lib/Request.php | 4 +++- tpl/bootstrap.php | 2 +- tpl/page.php | 2 +- 5 files changed, 7 insertions(+), 5 deletions(-) diff --git a/js/privatebin.js b/js/privatebin.js index 57eb273f..08aea83b 100644 --- a/js/privatebin.js +++ b/js/privatebin.js @@ -745,7 +745,7 @@ jQuery.PrivateBin = (function($, sjcl, Base64, RawDeflate) { { if (id === null) { // Attention: This also returns the delete token inside of the ID, if it is specified - id = window.location.search.substring(1); + id = (window.location.search.match(/[a-z0-9]{16}/) || [''])[0]; if (id === '') { throw 'no paste id given'; diff --git a/js/test/Prompt.js b/js/test/Prompt.js index 0e65b252..038f7a04 100644 --- a/js/test/Prompt.js +++ b/js/test/Prompt.js @@ -16,7 +16,7 @@ describe('Prompt', function () { 'string', function (password) { password = password.replace(/\r+/g, ''); - var clean = jsdom('', {url: 'ftp://example.com/?0'}); + var clean = jsdom('', {url: 'ftp://example.com/?0000000000000000'}); $('body').html( '