CVE-2016-9285: Exposure of Sensitive Information to an Unauthorized Actor (CWE-200)
value: 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.
CWE description: The product exposes sensitive information to an actor that is not explicitly authorized to have access to that information.
CVSS v3 base score: 5.3
Published: 2016-11-11T22:59Z
Repository: https://github.com/exponentcms/exponent-cms
Language: PHP
Files changed: framework/modules/addressbook/controllers/addressController.php
Commit date: 2016-11-03 10:04:48 -0400
Commit message: fix security vulnerability to editing addresses, countries, and regions; reported by pang0lin

Diff: diff --git a/framework/modules/addressbook/controllers/addressController.php b/framework/modules/addressbook/controllers/addressController.php
index e9825833ceb..a874da8735d 100755
--- a/framework/modules/addressbook/controllers/addressController.php
+++ b/framework/modules/addressbook/controllers/addressController.php
@@ -30,13 +30,20 @@ class addressController extends expController {
         'edit',
         'delete'
     );
     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(
         'aggregation',
         'categories',
@@ -63,12 +70,22 @@ class addressController extends expController {
         $this->myaddressbook();
 	}
 
     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,
             'admin_config'=>$config
         ));
@@ -81,11 +98,11 @@ class addressController extends expController {
 
 	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']);
         }
 		expHistory::set('viewable', $this->params);
 		$userid = (empty($this->params['user_id'])) ? $user->id : $this->params['user_id'];

Vulnerable code:         'process' => 'Import External Addresses'
        if((isset($this->params['id']))) $record = new address(intval($this->params['id']));
        else $record = null;
		expQueue::flashIfNotLoggedIn('message',gt('You must be logged in to manage your address book.'));
Fixed code:         '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',
        'edit'=>'You must be logged in to perform this action',
        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();
        }

		expQueue::flashIfNotLoggedIn('message',gt('You must be logged in to manage your address book.'));  //fixme is this redundant to common routine?