Vulnerability: framework/modules/addressbook/controllers/addressController.php in Exponent CMS v2.4.0 allows remote attackers to read user information via a modified id number, as demonstrated by address/edit/id/1, related to an "addresses, countries, and regions" issue.
Vulnerability ID: CVE-2016-9285
CWE: CWE-200
Severity score: 2.9
Project: https://github.com/exponentcms/exponent-cms
Published: 2016-11-11
File: framework/modules/addressbook/controllers/addressController.php
Commit message: fix security vulnerability to editing addresses, countries, and regions; reported by pang0lin
Commit author: dleffler

Vulnerable version (before fix):
 class addressController extends expController {     );     protected $manage_permissions = array( //        'import' => 'Import External Addresses',         'process' => 'Import External Addresses'     );     public $requires_login = array(         'myaddressbook'=>'You must be logged in to perform this action',     ); 	public $remove_configs = array( function showall() {      public function edit()     {         if((isset($this->params['id']))) $record = new address(intval($this->params['id']));         else $record = null;         $config = ecomconfig::getConfig('address_allow_admins_all');         assign_to_template(array(             'record'=>$record, public function myaddressbook() { 		global $user;  		// check if the user is logged in. 		expQueue::flashIfNotLoggedIn('message',gt('You must be logged in to manage your address book.'));         if (!$user->isAdmin() && $this->params['user_id'] != $user->id) {             unset($this->params['user_id']);         }

Fixed version (after fix):
 class addressController extends expController {     );     protected $manage_permissions = array( //        'import' => 'Import External Addresses',         'process' => 'Import External Addresses',         'edit_country' => 'Edit Country',         'delete_country' => 'Delete Country',         'update_country' => 'Update Country',         'edit_region' => 'Edit Region',         'delete_region' => 'Delete Region',         'update_region' => 'Update Region',     );     public $requires_login = array(         'edit'=>'You must be logged in to perform this action',         'myaddressbook'=>'You must be logged in to perform this action',     ); 	public $remove_configs = array( function showall() {      public function edit()     {         global $user;          $id = !empty($this->params['id']) ? $this->params['id'] : null;          // check to see if we should be editing.  You either need to be an admin, or editing own account.         if ($user->isAdmin() || ($user->id == $id)) {             $record = new address($id);         } else {             flash('error', gt('You do not have the proper permissions to edit this address'));             expHistory::back();         }          $config = ecomconfig::getConfig('address_allow_admins_all');         assign_to_template(array(             'record'=>$record, public function myaddressbook() { 		global $user;  		// check if the user is logged in. 		expQueue::flashIfNotLoggedIn('message',gt('You must be logged in to manage your address book.'));  //fixme is this redundant to common routine?         if (!$user->isAdmin() && $this->params['user_id'] != $user->id) {             unset($this->params['user_id']);         }

Patch:
patch: @@ -32,9 +32,16 @@ class addressController extends expController {
     );
     protected $manage_permissions = array(
 //        'import' => 'Import External Addresses',
-        'process' => 'Import External Addresses'
+        'process' => 'Import External Addresses',
+        'edit_country' => 'Edit Country',
+        'delete_country' => 'Delete Country',
+        'update_country' => 'Update Country',
+        'edit_region' => 'Edit Region',
+        'delete_region' => 'Delete Region',
+        'update_region' => 'Update Region',
     );
     public $requires_login = array(
+        'edit'=>'You must be logged in to perform this action',
         'myaddressbook'=>'You must be logged in to perform this action',
     );
 	public $remove_configs = array(
@@ -65,8 +72,18 @@ function showall() {
 
     public function edit()
     {
-        if((isset($this->params['id']))) $record = new address(intval($this->params['id']));
-        else $record = null;
+        global $user;
+
+        $id = !empty($this->params['id']) ? $this->params['id'] : null;
+
+        // check to see if we should be editing.  You either need to be an admin, or editing own account.
+        if ($user->isAdmin() || ($user->id == $id)) {
+            $record = new address($id);
+        } else {
+            flash('error', gt('You do not have the proper permissions to edit this address'));
+            expHistory::back();
+        }
+
         $config = ecomconfig::getConfig('address_allow_admins_all');
         assign_to_template(array(
             'record'=>$record,
@@ -83,7 +100,7 @@ public function myaddressbook() {
 		global $user;
 
 		// check if the user is logged in.
-		expQueue::flashIfNotLoggedIn('message',gt('You must be logged in to manage your address book.'));
+		expQueue::flashIfNotLoggedIn('message',gt('You must be logged in to manage your address book.'));  //fixme is this redundant to common routine?
         if (!$user->isAdmin() && $this->params['user_id'] != $user->id) {
             unset($this->params['user_id']);
         }
path: framework/modules/addressbook/controllers/addressController.php