From a14bb9e843705c74575a9c2d9fb82eb0b9f3ae6c Mon Sep 17 00:00:00 2001 From: minhaz Date: Mon, 18 Sep 2017 02:26:33 +0530 Subject: [PATCH 1/5] fix for issue 80 https://github.com/mebjas/CSRF-Protector-PHP/issues/80 Added support for content-type = application json in both client and server side. --- js/csrfprotector.js | 31 ++++++++++++++++++++++++++++++- libs/csrf/csrfprotector.php | 31 ++++++++++++++++++++++++++----- 2 files changed, 56 insertions(+), 6 deletions(-) diff --git a/js/csrfprotector.js b/js/csrfprotector.js index aa548cb..74d1e6a 100644 --- a/js/csrfprotector.js +++ b/js/csrfprotector.js @@ -290,8 +290,22 @@ function csrfprotector_init() { function new_send(data) { if (this.method.toLowerCase() === 'post') { if (data !== null && typeof data === 'object') { - data.append(CSRFP.CSRFP_TOKEN, CSRFP._getAuthKey()); + data[CSRFP.CSRFP_TOKEN] = CSRFP._getAuthKey(); } else { + // Added support for content type == application / json + if (this.headers && 'Content-Type' in this.headers + && this.headers['Content-Type'] === 'application/json') { + try { + data = JSON.parse(data) + data[CSRFP.CSRFP_TOKEN] = CSRFP._getAuthKey(); + return this.old_send(JSON.stringify(data)); + + } catch (ex) { + console.log("[ERROR] [CSRF Protector] Unable to parse content ", + "when content-type is application/json", ex); + } + } + if (typeof data != "undefined") { data += "&"; } else { @@ -303,12 +317,27 @@ function csrfprotector_init() { return this.old_send(data); } + /** + * Wrapper method to override setRequestHeader method of + * XMLHttpRequests + * @param: header - header name + * @param: value - header value + */ + function new_setRequestHeader(header, value) { + if (!this.headers) this.headers = {}; + this.headers[header] = value; + + this.old_setRequestHeader(header, value); + } + if (window.XMLHttpRequest) { // Wrapping XMLHttpRequest.prototype.old_send = XMLHttpRequest.prototype.send; XMLHttpRequest.prototype.old_open = XMLHttpRequest.prototype.open; + XMLHttpRequest.prototype.old_setRequestHeader = XMLHttpRequest.prototype.setRequestHeader; XMLHttpRequest.prototype.open = new_open; XMLHttpRequest.prototype.send = new_send; + XMLHttpRequest.prototype.setRequestHeader = new_setRequestHeader; } if (typeof ActiveXObject !== 'undefined') { ActiveXObject.prototype.old_send = ActiveXObject.prototype.send; diff --git a/libs/csrf/csrfprotector.php b/libs/csrf/csrfprotector.php index ca0fafd..9d1439e 100755 --- a/libs/csrf/csrfprotector.php +++ b/libs/csrf/csrfprotector.php @@ -25,6 +25,11 @@ class alreadyInitializedException extends \exception {}; class csrfProtector { + /* + * application/json content type + */ + const JSONCONTENTTYPE = "application/json"; + /* * Variable: $cookieExpiryTime * expiry time for cookie @@ -195,14 +200,30 @@ public static function authorizePost() //set request type to POST self::$requestType = "POST"; + $token = (isset($_POST[self::$config['CSRFP_TOKEN']])) + ? $_POST[self::$config['CSRFP_TOKEN']] : false; + + if ($_SERVER["CONTENT_TYPE"] === self::JSONCONTENTTYPE) { + try { + $request_body = file_get_contents('php://input'); + $request_body = json_decode($request_body, true); + if (isset($request_body[self::$config['CSRFP_TOKEN']])) { + $token = $request_body[self::$config['CSRFP_TOKEN']]; + } + } catch (Exception $ex) { + // silently absorb this exception + // it could be because IO is blocked or json decode fails + // either way log it or add some handleing + // TODO ^^ + } + } + //currently for same origin only - if (!(isset($_POST[self::$config['CSRFP_TOKEN']]) - && isset($_SESSION[self::$config['CSRFP_TOKEN']]) - && (self::isValidToken($_POST[self::$config['CSRFP_TOKEN']])) - )) { + if (!($token && isset($_SESSION[self::$config['CSRFP_TOKEN']]) + && (self::isValidToken($token)))) { //action in case of failed validation - self::failedValidationAction(); + self::failedValidationAction(); } else { self::refreshToken(); //refresh token for successfull validation } From d0017ad68b8a570633ddb257c768517d252f5332 Mon Sep 17 00:00:00 2001 From: minhaz Date: Sun, 24 Sep 2017 04:32:09 +0530 Subject: [PATCH 2/5] transport of token moved to headers for XHR post On js side, token is added to request header in case of POST request. On server side - POST request, it looks for data in header, if failed then looks for data in post payload. --- js/csrfprotector.js | 41 ++----------------------------------- libs/csrf/csrfprotector.php | 26 ++++++++--------------- 2 files changed, 11 insertions(+), 56 deletions(-) diff --git a/js/csrfprotector.js b/js/csrfprotector.js index 74d1e6a..0d22955 100644 --- a/js/csrfprotector.js +++ b/js/csrfprotector.js @@ -289,55 +289,18 @@ function csrfprotector_init() { */ function new_send(data) { if (this.method.toLowerCase() === 'post') { - if (data !== null && typeof data === 'object') { - data[CSRFP.CSRFP_TOKEN] = CSRFP._getAuthKey(); - } else { - // Added support for content type == application / json - if (this.headers && 'Content-Type' in this.headers - && this.headers['Content-Type'] === 'application/json') { - try { - data = JSON.parse(data) - data[CSRFP.CSRFP_TOKEN] = CSRFP._getAuthKey(); - return this.old_send(JSON.stringify(data)); - - } catch (ex) { - console.log("[ERROR] [CSRF Protector] Unable to parse content ", - "when content-type is application/json", ex); - } - } - - if (typeof data != "undefined") { - data += "&"; - } else { - data = ""; - } - data += CSRFP.CSRFP_TOKEN +"=" +CSRFP._getAuthKey(); - } + // attach the token in request header + this.setRequestHeader(CSRFP.CSRFP_TOKEN, CSRFP._getAuthKey()); } return this.old_send(data); } - /** - * Wrapper method to override setRequestHeader method of - * XMLHttpRequests - * @param: header - header name - * @param: value - header value - */ - function new_setRequestHeader(header, value) { - if (!this.headers) this.headers = {}; - this.headers[header] = value; - - this.old_setRequestHeader(header, value); - } - if (window.XMLHttpRequest) { // Wrapping XMLHttpRequest.prototype.old_send = XMLHttpRequest.prototype.send; XMLHttpRequest.prototype.old_open = XMLHttpRequest.prototype.open; - XMLHttpRequest.prototype.old_setRequestHeader = XMLHttpRequest.prototype.setRequestHeader; XMLHttpRequest.prototype.open = new_open; XMLHttpRequest.prototype.send = new_send; - XMLHttpRequest.prototype.setRequestHeader = new_setRequestHeader; } if (typeof ActiveXObject !== 'undefined') { ActiveXObject.prototype.old_send = ActiveXObject.prototype.send; diff --git a/libs/csrf/csrfprotector.php b/libs/csrf/csrfprotector.php index 9d1439e..81c5871 100755 --- a/libs/csrf/csrfprotector.php +++ b/libs/csrf/csrfprotector.php @@ -1,5 +1,4 @@ Date: Sun, 24 Sep 2017 05:12:53 +0530 Subject: [PATCH 3/5] added support to case when apache_request_headers is missing server now looks for token in payload else from header. In case of header first from - apache_request_headers else from superglobal $_SERVER --- libs/csrf/csrfprotector.php | 40 ++++++++++++++++++++++++++++++------- 1 file changed, 33 insertions(+), 7 deletions(-) diff --git a/libs/csrf/csrfprotector.php b/libs/csrf/csrfprotector.php index 81c5871..e084f6e 100755 --- a/libs/csrf/csrfprotector.php +++ b/libs/csrf/csrfprotector.php @@ -201,13 +201,8 @@ public static function authorizePost() //set request type to POST self::$requestType = "POST"; - // look for token in header, then in payload - $token = false; - if (isset(apache_request_headers()[self::$config['CSRFP_TOKEN']])) { - $token = apache_request_headers()[self::$config['CSRFP_TOKEN']]; - } else if (isset($_POST[self::$config['CSRFP_TOKEN']])) { - $token = $_POST[self::$config['CSRFP_TOKEN']]; - } + // look for token in payload else from header + $token = self::getTokenFromRequest(); //currently for same origin only if (!($token && isset($_SESSION[self::$config['CSRFP_TOKEN']]) @@ -234,6 +229,37 @@ public static function authorizePost() } } + /* + * Fucntion: getTokenFromRequest + * function to get token in case of POST request + * + * Parameters: + * void + * + * Returns: + * any (string / bool) - token retrieved from header or form payload + */ + private static function getTokenFromRequest() { + // look for token in header, then in payload + if (isset($_POST[self::$config['CSRFP_TOKEN']])) { + return $_POST[self::$config['CSRFP_TOKEN']]; + } + + if (function_exists('apache_request_headers')) { + if (isset(apache_request_headers()[self::$config['CSRFP_TOKEN']])) { + return apache_request_headers()[self::$config['CSRFP_TOKEN']]; + } + } else { + $serverKey = 'HTTP_' .strtoupper(self::$config['CSRFP_TOKEN']); + $serverKey = str_replace('-', '_', $serverKey); + if (isset($_SERVER[$serverKey])) { + return $_SERVER[$serverKey]; + } + } + + return false; + } + /* * Function: isValidToken * function to check the validity of token in session array From c756bf3250c6c8c249fcc36817d9ef33700305e0 Mon Sep 17 00:00:00 2001 From: minhaz Date: Fri, 6 Oct 2017 02:18:29 +0530 Subject: [PATCH 4/5] travis yaml for build failure fixes, from master --- .travis.yml | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/.travis.yml b/.travis.yml index d5821b2..dc659f4 100644 --- a/.travis.yml +++ b/.travis.yml @@ -3,16 +3,18 @@ php: - "5.6" - "5.5" - "5.4" - - "5.3" - "7.0" - "7.1" - hhvm - nightly + - "5.3" + matrix: allow_failures: - php: nightly - php: hhvm + - php: "5.3" os: - linux @@ -43,4 +45,4 @@ after_success: cache: directories: - vendor - - $HOME/.cache/composer + - $HOME/.cache/composer \ No newline at end of file From 5fda1dcddc8f4fc65c428dba4f926a8cfc9c1cfe Mon Sep 17 00:00:00 2001 From: minhaz Date: Fri, 6 Oct 2017 06:15:47 +0530 Subject: [PATCH 5/5] Added more test cases, some logic fixes --- libs/csrf/csrfprotector.php | 33 +++++++++++++++++---------------- test/csrfprotector_test.php | 31 +++++++++++++++++++++++++++++-- 2 files changed, 46 insertions(+), 18 deletions(-) diff --git a/libs/csrf/csrfprotector.php b/libs/csrf/csrfprotector.php index 13cb17c..7e097a2 100755 --- a/libs/csrf/csrfprotector.php +++ b/libs/csrf/csrfprotector.php @@ -65,11 +65,6 @@ function __construct($cfg) { class csrfProtector { - /* - * application/json content type - */ - const JSONCONTENTTYPE = "application/json"; - /* * Variable: $cookieExpiryTime * expiry time for cookie @@ -94,10 +89,17 @@ class csrfProtector /** * Variable: $cookieConfig * Array of parameters for the setcookie method - * @var cookieConfig; + * @var array */ private static $cookieConfig = null; + /** + * Variable: $tokenHeaderKey + * Key value in header array, which contain the token + * @var string + */ + private static $tokenHeaderKey = null; + /* * Variable: $requestType * Varaible to store weather request type is post or get @@ -192,6 +194,9 @@ public static function init($length = null, $action = null) if (self::$config['CSRFP_TOKEN'] == '') self::$config['CSRFP_TOKEN'] = CSRFP_TOKEN; + self::$tokenHeaderKey = 'HTTP_' .strtoupper(self::$config['CSRFP_TOKEN']); + self::$tokenHeaderKey = str_replace('-', '_', self::$tokenHeaderKey); + // load parameters for setcookie method if (!isset(self::$config['cookieConfig'])) self::$config['cookieConfig'] = array(); @@ -211,8 +216,6 @@ public static function init($length = null, $action = null) } } - // TODO: initialize the setcookie params, based on config; - // Authorise the incoming request self::authorizePost(); @@ -267,7 +270,6 @@ public static function authorizePost() self::refreshToken(); //refresh token for successfull validation } } else if (!static::isURLallowed()) { - //currently for same origin only if (!(isset($_GET[self::$config['CSRFP_TOKEN']]) && isset($_SESSION[self::$config['CSRFP_TOKEN']]) @@ -293,7 +295,7 @@ public static function authorizePost() * any (string / bool) - token retrieved from header or form payload */ private static function getTokenFromRequest() { - // look for token in header, then in payload + // look for in $_POST, then header if (isset($_POST[self::$config['CSRFP_TOKEN']])) { return $_POST[self::$config['CSRFP_TOKEN']]; } @@ -302,12 +304,11 @@ private static function getTokenFromRequest() { if (isset(apache_request_headers()[self::$config['CSRFP_TOKEN']])) { return apache_request_headers()[self::$config['CSRFP_TOKEN']]; } - } else { - $serverKey = 'HTTP_' .strtoupper(self::$config['CSRFP_TOKEN']); - $serverKey = str_replace('-', '_', $serverKey); - if (isset($_SERVER[$serverKey])) { - return $_SERVER[$serverKey]; - } + } + + if (self::$tokenHeaderKey === null) return false; + if (isset($_SERVER[self::$tokenHeaderKey])) { + return $_SERVER[self::$tokenHeaderKey]; } return false; diff --git a/test/csrfprotector_test.php b/test/csrfprotector_test.php index 388ff9a..52a1279 100644 --- a/test/csrfprotector_test.php +++ b/test/csrfprotector_test.php @@ -350,11 +350,10 @@ public function testAuthorisePost_failedAction_6() } /** - * test authorise success + * test authorise success with token in $_POST */ public function testAuthorisePost_success() { - $_SERVER['REQUEST_METHOD'] = 'POST'; $_POST[csrfprotector::$config['CSRFP_TOKEN']] = $_GET[csrfprotector::$config['CSRFP_TOKEN']] @@ -382,6 +381,34 @@ public function testAuthorisePost_success() // $this->assertTrue(csrfp_wrapper::checkHeader($_SESSION[csrfprotector::$config['CSRFP_TOKEN']][0])); // Combine these 3 later } + /** + * test authorise success with token in header + */ + public function testAuthorisePost_success_2() + { + unset($_POST[csrfprotector::$config['CSRFP_TOKEN']]); + $_SERVER['REQUEST_METHOD'] = 'POST'; + $serverKey = 'HTTP_' .strtoupper(csrfprotector::$config['CSRFP_TOKEN']); + $serverKey = str_replace('-', '_', $serverKey); + + $csrfp = new csrfProtector; + $reflection = new \ReflectionClass(get_class($csrfp)); + $property = $reflection->getProperty('tokenHeaderKey'); + $property->setAccessible(true); + // change value to false + $property->setValue($csrfp, $serverKey); + + $_SERVER[$serverKey] = $_SESSION[csrfprotector::$config['CSRFP_TOKEN']][0]; + $temp = $_SESSION[csrfprotector::$config['CSRFP_TOKEN']]; + + csrfprotector::authorizePost(); //will create new session and cookies + $this->assertFalse($temp == $_SESSION[csrfprotector::$config['CSRFP_TOKEN']][0]); + $this->assertTrue(csrfp_wrapper::checkHeader('Set-Cookie')); + $this->assertTrue(csrfp_wrapper::checkHeader('csrfp_token')); + // $this->assertTrue(csrfp_wrapper::checkHeader($_SESSION[csrfprotector::$config['CSRFP_TOKEN']][0])); // Combine these 3 later + + } + /** * test for generateAuthToken() */