From 086cc31982887ee6c2820cf2badf2eba043f995e Mon Sep 17 00:00:00 2001 From: davidcallizaya Date: Fri, 13 Oct 2017 07:57:22 -0400 Subject: [PATCH 1/2] HOR-3921 Fix CSRF security issue. --- workflow/engine/PmBootstrap.php | 3 +- .../engine/methods/login/authentication.php | 6 +- .../methods/login/authenticationSso.php | 7 +- .../processes/processes_Import_Bpmn.php | 1 - .../services/webentry/anonymousLogin.php | 3 +- workflow/engine/methods/users/usersAjax.php | 1 + workflow/engine/skinEngine/skinEngine.php | 3 + .../engine/src/ProcessMaker/Util/helpers.php | 34 ++++++ workflow/engine/templates/users/users.js | 7 ++ workflow/public_html/bootstrap.php | 3 +- workflow/public_html/pmGmail/sso.php | 106 +++++++++--------- workflow/public_html/sysGeneric.php | 6 +- 12 files changed, 112 insertions(+), 68 deletions(-) diff --git a/workflow/engine/PmBootstrap.php b/workflow/engine/PmBootstrap.php index 8c54c057c..0d1e5870e 100644 --- a/workflow/engine/PmBootstrap.php +++ b/workflow/engine/PmBootstrap.php @@ -323,8 +323,7 @@ class PmBootstrap extends Bootstrap require_once 'classes/model/Users.php'; $oUser = new Users(); $aUser = $oUser->load($aSession['USR_UID']); - $_SESSION['USER_LOGGED'] = $aUser['USR_UID']; - $_SESSION['USR_USERNAME'] = $aUser['USR_USERNAME']; + initUserSession($aUser['USR_UID'], $aUser['USR_USERNAME']); $bRedirect = false; $RBAC->initRBAC(); $RBAC->loadUserRolePermission( $RBAC->sSystem, $_SESSION['USER_LOGGED'] ); diff --git a/workflow/engine/methods/login/authentication.php b/workflow/engine/methods/login/authentication.php index 6934fe4ca..d6abdd619 100644 --- a/workflow/engine/methods/login/authentication.php +++ b/workflow/engine/methods/login/authentication.php @@ -182,14 +182,12 @@ try { $oPluginRegistry->executeTriggers ( PM_LOGIN , $loginInfo ); } EnterpriseClass::enterpriseSystemUpdate($loginInfo); - $_SESSION['USER_LOGGED'] = $uid; - $_SESSION['USR_USERNAME'] = $usr; + initUserSession($uid, $usr); } else { setcookie("singleSignOn", '1', time() + (24 * 60 * 60), '/'); $uid = $RBAC->userObj->fields['USR_UID']; $usr = $RBAC->userObj->fields['USR_USERNAME']; - $_SESSION['USER_LOGGED'] = $uid; - $_SESSION['USR_USERNAME'] = $usr; + initUserSession($uid, $usr); } //Set default Languaje diff --git a/workflow/engine/methods/login/authenticationSso.php b/workflow/engine/methods/login/authenticationSso.php index 1d8a9cafb..efb6935a5 100644 --- a/workflow/engine/methods/login/authenticationSso.php +++ b/workflow/engine/methods/login/authenticationSso.php @@ -129,9 +129,10 @@ try { setcookie('singleSignOn', '1', time() + (24 * 60 * 60), '/'); - $_SESSION['USER_LOGGED'] = $_SESSION['__USER_LOGGED_SSO__']; - $_SESSION['USR_USERNAME'] = $_SESSION['__USR_USERNAME_SSO__']; - + initUserSession( + $_SESSION['__USER_LOGGED_SSO__'], + $_SESSION['__USR_USERNAME_SSO__'] + ); unset($_SESSION['__USER_LOGGED_SSO__'], $_SESSION['__USR_USERNAME_SSO__']); G::header('Location: ' . $location); diff --git a/workflow/engine/methods/processes/processes_Import_Bpmn.php b/workflow/engine/methods/processes/processes_Import_Bpmn.php index 8e21eb4bd..42733fa65 100644 --- a/workflow/engine/methods/processes/processes_Import_Bpmn.php +++ b/workflow/engine/methods/processes/processes_Import_Bpmn.php @@ -4,7 +4,6 @@ ini_set("max_execution_time", 0); $filter = new InputFilter(); $_FILES = $filter->xssFilterHard($_FILES); -$_SESSION['USER_LOGGED'] = $filter->xssFilterHard($_SESSION['USER_LOGGED']); if (isset($_FILES["PROCESS_FILENAME"]) && pathinfo($_FILES["PROCESS_FILENAME"]["name"], PATHINFO_EXTENSION) == "bpmn" diff --git a/workflow/engine/methods/services/webentry/anonymousLogin.php b/workflow/engine/methods/services/webentry/anonymousLogin.php index 2b8387149..208cee429 100644 --- a/workflow/engine/methods/services/webentry/anonymousLogin.php +++ b/workflow/engine/methods/services/webentry/anonymousLogin.php @@ -24,8 +24,7 @@ try { throw new \Exception('WebEntry User not found'); } - $_SESSION['USER_LOGGED'] = $userUid; - $_SESSION['USR_USERNAME'] = $userInfo['username']; + initUserSession($userUid, $userInfo['username']); $result = [ 'user_logged' => $userUid, diff --git a/workflow/engine/methods/users/usersAjax.php b/workflow/engine/methods/users/usersAjax.php index 9c97c46ca..2532da641 100644 --- a/workflow/engine/methods/users/usersAjax.php +++ b/workflow/engine/methods/users/usersAjax.php @@ -129,6 +129,7 @@ switch ($_POST['action']) { case 'saveUser': case 'savePersonalInfo': try { + verifyCsrfToken($_POST); $user = new \ProcessMaker\BusinessModel\User(); $form = $_POST; $permissionsToSaveData = $user->getPermissionsForEdit(); diff --git a/workflow/engine/skinEngine/skinEngine.php b/workflow/engine/skinEngine/skinEngine.php index 61f0e37a4..1174d189b 100644 --- a/workflow/engine/skinEngine/skinEngine.php +++ b/workflow/engine/skinEngine/skinEngine.php @@ -261,9 +261,11 @@ class SkinEngine $template = new TemplatePower($templateFile); $template->prepare(); + $header = '' . "\n" . $header; $template->assign('header', $header); $template->assign('styles', $styles); $template->assign('bodyTemplate', $body); + $template->assign('csrf_token', csrfToken()); $doctype = ""; $meta = null; @@ -569,6 +571,7 @@ class SkinEngine $smarty->cache_dir = PATH_SMARTY_CACHE; $smarty->config_dir = PATH_THIRDPARTY . 'smarty/configs'; $smarty->register_function('translate', 'translate'); + $smarty->register_function('csrf_token', 'csrfToken'); $viewVars = $oHeadPublisher->getVars(); diff --git a/workflow/engine/src/ProcessMaker/Util/helpers.php b/workflow/engine/src/ProcessMaker/Util/helpers.php index 828ef8e8d..90747d11f 100644 --- a/workflow/engine/src/ProcessMaker/Util/helpers.php +++ b/workflow/engine/src/ProcessMaker/Util/helpers.php @@ -1,4 +1,8 @@ load($aSession['USR_UID']); - $_SESSION['USER_LOGGED'] = $aUser['USR_UID']; - $_SESSION['USR_USERNAME'] = $aUser['USR_USERNAME']; + initUserSession($aUser['USR_UID'], $aUser['USR_USERNAME']); $bRedirect = false; if (PHP_VERSION < 5.2) { setcookie(session_name(), session_id(), time() + $timelife, '/', '; HttpOnly'); diff --git a/workflow/public_html/pmGmail/sso.php b/workflow/public_html/pmGmail/sso.php index b6958096f..346c6a701 100644 --- a/workflow/public_html/pmGmail/sso.php +++ b/workflow/public_html/pmGmail/sso.php @@ -15,7 +15,7 @@ $server = isset($_GET['server']) ? $_GET['server'] : ''; //We do need the server to continue. if( !isset($_GET['server']) || $server == "" ){ - throw new \Exception(Bootstrap::LoadTranslation( 'ID_GMAIL_NEED_SERVER' )); + throw new \Exception(Bootstrap::LoadTranslation( 'ID_GMAIL_NEED_SERVER' )); } //First check if the feature is enabled in the license. @@ -53,75 +53,77 @@ curl_close($curl); $decodedResp = G::json_decode($curl_response); if(!is_object($decodedResp) || property_exists($decodedResp,'error')) { - die($decodedResp->error->message); + die($decodedResp->error->message); } //getting the enviroment $enviroment = $decodedResp->enviroment; if(count($decodedResp->user) > 1){ - echo Bootstrap::LoadTranslation( 'ID_EMAIL_MORE_THAN_ONE_USER' ); - die; + echo Bootstrap::LoadTranslation( 'ID_EMAIL_MORE_THAN_ONE_USER' ); + die; } else if(count($decodedResp->user) < 1){ - echo Bootstrap::LoadTranslation( 'ID_USER_NOT_FOUND' ); - die; + echo Bootstrap::LoadTranslation( 'ID_USER_NOT_FOUND' ); + die; } //validationg if there is an actual PM session if( !isset($_SESSION['USER_LOGGED']) || $_SESSION['USER_LOGGED'] != $decodedResp->user['0']->USR_UID){ - $url = 'https://www.googleapis.com/oauth2/v1/tokeninfo?access_token='.$gmailToken; + $url = 'https://www.googleapis.com/oauth2/v1/tokeninfo?access_token='.$gmailToken; - // init curl object - $ch = curl_init(); - // define options - $optArray = array( + // init curl object + $ch = curl_init(); + // define options + $optArray = array( CURLOPT_URL => $url, CURLOPT_RETURNTRANSFER => true, CURLOPT_SSL_VERIFYPEER => false, CURLOPT_SSL_VERIFYHOST => false - ); - // apply those options - curl_setopt_array($ch, $optArray); - // execute request and get response - $result = curl_exec($ch); - $response = (G::json_decode($result)); - curl_close($ch); + ); + // apply those options + curl_setopt_array($ch, $optArray); + // execute request and get response + $result = curl_exec($ch); + $response = (G::json_decode($result)); + curl_close($ch); - //First validate if this user (mail) corresponds to a PM user - if(isset($response->email) && ($gmail == $response->email)){ - //If the email corresponds I get the username and with the gmail user_id the session is created. - if($decodedResp->user['0']->USR_STATUS == "ACTIVE"){ - //User Active! lets create the Session - @session_destroy(); - session_start(); - session_regenerate_id(); - - if (PHP_VERSION < 5.2) { - setcookie("workspaceSkin", $enviroment, time() + (24 * 60 * 60), "/sys" . $enviroment, "; HttpOnly"); - } else { - setcookie("workspaceSkin", $enviroment, time() + (24 * 60 * 60), "/sys" . $enviroment, null, false, true); - } + //First validate if this user (mail) corresponds to a PM user + if(isset($response->email) && ($gmail == $response->email)){ + //If the email corresponds I get the username and with the gmail user_id the session is created. + if($decodedResp->user['0']->USR_STATUS == "ACTIVE"){ + //User Active! lets create the Session + @session_destroy(); + session_start(); + session_regenerate_id(); - $_SESSION = array(); - $_SESSION['__EE_INSTALLATION__'] = 2; - $_SESSION['__EE_SW_PMLICENSEMANAGER__'] = 1; - $_SESSION['phpLastFileFound'] = ''; - $_SESSION['USERNAME_PREVIOUS1'] = $decodedResp->user['0']->USR_USERNAME; - $_SESSION['USERNAME_PREVIOUS2'] = $decodedResp->user['0']->USR_USERNAME; - $_SESSION['WORKSPACE'] = $pmws; - $_SESSION['USER_LOGGED'] = $decodedResp->user['0']->USR_UID; - $_SESSION['USR_USERNAME'] = $decodedResp->user['0']->USR_USERNAME; - $_SESSION['USR_FULLNAME'] = $decodedResp->user['0']->USR_FIRSTNAME. ' ' .$decodedResp->user['0']->USR_LASTNAME; - $_SESSION['__sw__'] = 1; - //session created - } else { - echo Bootstrap::LoadTranslation( 'ID_USER_NOT_ACTIVE' ); - die; - } - } else { - echo Bootstrap::LoadTranslation( 'ID_USER_DOES_NOT_CORRESPOND' ); - die; - } + if (PHP_VERSION < 5.2) { + setcookie("workspaceSkin", $enviroment, time() + (24 * 60 * 60), "/sys" . $enviroment, "; HttpOnly"); + } else { + setcookie("workspaceSkin", $enviroment, time() + (24 * 60 * 60), "/sys" . $enviroment, null, false, true); + } + + $_SESSION = array(); + $_SESSION['__EE_INSTALLATION__'] = 2; + $_SESSION['__EE_SW_PMLICENSEMANAGER__'] = 1; + $_SESSION['phpLastFileFound'] = ''; + $_SESSION['USERNAME_PREVIOUS1'] = $decodedResp->user['0']->USR_USERNAME; + $_SESSION['USERNAME_PREVIOUS2'] = $decodedResp->user['0']->USR_USERNAME; + $_SESSION['WORKSPACE'] = $pmws; + $_SESSION['USR_FULLNAME'] = $decodedResp->user['0']->USR_FIRSTNAME. ' ' .$decodedResp->user['0']->USR_LASTNAME; + $_SESSION['__sw__'] = 1; + initUserSession( + $decodedResp->user['0']->USR_UID, + $decodedResp->user['0']->USR_USERNAME + ); + //session created + } else { + echo Bootstrap::LoadTranslation( 'ID_USER_NOT_ACTIVE' ); + die; + } + } else { + echo Bootstrap::LoadTranslation( 'ID_USER_DOES_NOT_CORRESPOND' ); + die; + } } $_SESSION['server'] = 'https://' . $server . '/sys'. $pmws .'/en/'.$enviroment.'/'; diff --git a/workflow/public_html/sysGeneric.php b/workflow/public_html/sysGeneric.php index d61fbc598..8b18f5c18 100644 --- a/workflow/public_html/sysGeneric.php +++ b/workflow/public_html/sysGeneric.php @@ -979,8 +979,10 @@ if (! defined( 'EXECUTE_BY_CRON' )) { require_once 'classes/model/Users.php'; $oUser = new Users(); $aUser = $oUser->load( $aSession['USR_UID'] ); - $_SESSION['USER_LOGGED'] = $aUser['USR_UID']; - $_SESSION['USR_USERNAME'] = $aUser['USR_USERNAME']; + initUserSession( + $_SESSION['USER_LOGGED'], + $aUser['USR_USERNAME'] + ); $bRedirect = false; if ((preg_match("/msie/i", $_SERVER ['HTTP_USER_AGENT']) != 1 || $config['ie_cookie_lifetime'] == 1) && From 69b2370ba6f6ce0284ecb703647a08d3326e6131 Mon Sep 17 00:00:00 2001 From: davidcallizaya Date: Fri, 13 Oct 2017 10:49:45 -0400 Subject: [PATCH 2/2] HOR-3921 Fix CR observations. --- workflow/engine/skinEngine/skinEngine.php | 2 +- workflow/engine/src/ProcessMaker/Util/helpers.php | 13 +++++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/workflow/engine/skinEngine/skinEngine.php b/workflow/engine/skinEngine/skinEngine.php index 1174d189b..a774506ed 100644 --- a/workflow/engine/skinEngine/skinEngine.php +++ b/workflow/engine/skinEngine/skinEngine.php @@ -261,7 +261,7 @@ class SkinEngine $template = new TemplatePower($templateFile); $template->prepare(); - $header = '' . "\n" . $header; + $header = '' . "\n" . $header; $template->assign('header', $header); $template->assign('styles', $styles); $template->assign('bodyTemplate', $body); diff --git a/workflow/engine/src/ProcessMaker/Util/helpers.php b/workflow/engine/src/ProcessMaker/Util/helpers.php index 90747d11f..cccfbb9ad 100644 --- a/workflow/engine/src/ProcessMaker/Util/helpers.php +++ b/workflow/engine/src/ProcessMaker/Util/helpers.php @@ -376,6 +376,12 @@ function initUserSession($usrUid, $usrName) $_SESSION['USR_CSRF_TOKEN'] = Str::random(40); } +/** + * Verify token for an incoming request. + * + * @param type $request + * @throws TokenMismatchException + */ function verifyCsrfToken($request) { $headers = getallheaders(); @@ -386,11 +392,18 @@ function verifyCsrfToken($request) : null); $match = is_string($_SESSION['USR_CSRF_TOKEN']) && is_string($token) + && !empty($_SESSION['USR_CSRF_TOKEN']) && hash_equals($_SESSION['USR_CSRF_TOKEN'], $token); if (!$match) { throw new TokenMismatchException(); } } + +/** + * Get the current user CSRF token. + * + * @return string + */ function csrfToken() { return isset($_SESSION['USR_CSRF_TOKEN']) ? $_SESSION['USR_CSRF_TOKEN'] : '';