CVE-2022-1177: Incorrect Authorization (CWE-863)
value: Accounting User Can Download Patient Reports in openemr in GitHub repository openemr/openemr prior to 6.1.0.
CWE description: The product performs an authorization check when an actor attempts to access a resource or perform an action, but it does not correctly perform the check. This allows attackers to bypass intended access restrictions.
CVSS v3 base score: 4.3
Published: 2022-03-30T11:15Z
Repository: https://github.com/openemr/openemr
Language: PHP
Files changed: acl_upgrade.php, interface/patient_file/report/custom_report.php, library/classes/Installer.class.php, src/Common/Acl/AclExtended.php, src/Gacl/GaclApi.php, version.php
Commit date: 2022-02-14 10:25:10 -0500
Commit message: limit access to reports for front and back office (#4893)

* add acl report check to custom report

* remove report and notes access for accountants and front office on install

* revert change to administrators

* adjust install and upgrade

* reorder case in shiftAcl

* add alter acls to template

* review fixes, thanks Brady!

* more review fixes
Diff: diff --git a/acl_upgrade.php b/acl_upgrade.php
index 43d4fbcae9..6501e9c032 100644
--- a/acl_upgrade.php
+++ b/acl_upgrade.php
@@ -814,17 +814,52 @@ if ($acl_version < $upgrade_acl) {
 
     //DONE with upgrading to this version
     $acl_version = $upgrade_acl;
 }
 
-
-/* This is a template for a new revision, when needed
 // Upgrade for acl_version 12
 $upgrade_acl = 12;
 if ($acl_version < $upgrade_acl) {
     echo "<B>UPGRADING ACCESS CONTROLS TO VERSION " . $upgrade_acl . ":</B></BR>";
 
+    //Collect the ACL ID numbers.
+    echo "<B>Checking to ensure all the proper ACL(access control list) are present:</B></BR>";
+    $accounting_view = AclExtended::getAclIdNumber('Accounting', 'view');
+    $frontoffice_view = AclExtended::getAclIdNumber('Front Office', 'view');
+    $frontoffice_write = AclExtended::getAclIdNumber('Front Office', 'write');
+
+    //Add new object Sections
+    echo "<BR/><B>Adding new object sections</B><BR/>";
+
+    //Add new Objects
+    echo "<BR/><B>Adding new objects</B><BR/>";
+
+    //Update already existing Objects
+    echo "<BR/><B>Upgrading objects</B><BR/>";
+
+    //Add new ACLs here (will return the ACL ID of newly created or already existant ACL)
+    // (will also place in the appropriate group and CREATE a new group if needed)
+    echo "<BR/><B>Adding ACLs(Access Control Lists) and groups</B><BR/>";
+
+    //Update the ACLs
+    echo "<BR/><B>Updating the ACLs(Access Control Lists)</B><BR/>";
+    AclExtended::shiftAcl($accounting_view, 'Accounting', 'patients', 'Patients', 'pat_rep', 'Patient Report', 'view');
+    AclExtended::shiftAcl($frontoffice_view, 'Front Office', 'patients', 'Patients', 'pat_rep', 'Patient Report', 'view');
+    AclExtended::shiftAcl($frontoffice_write, 'Front Office', 'patients', 'Patients', 'trans', 'Transactions (write,wsome optional)', 'write');
+    AclExtended::shiftAcl($frontoffice_write, 'Front Office', 'patients', 'Patients', 'notes', 'Patient Notes (write,wsome optional)', 'write');
+
+
+    //DONE with upgrading to this version
+    $acl_version = $upgrade_acl;
+}
+
+/* This is a template for a new revision, when needed
+// Upgrade for acl_version 13
+$upgrade_acl = 13;
+if ($acl_version < $upgrade_acl) {
+    echo "<B>UPGRADING ACCESS CONTROLS TO VERSION " . $upgrade_acl . ":</B></BR>";
+
     //Collect the ACL ID numbers.
     echo "<B>Checking to ensure all the proper ACL(access control list) are present:</B></BR>";
 
     //Add new object Sections
     echo "<BR/><B>Adding new object sections</B><BR/>";
diff --git a/interface/patient_file/report/custom_report.php b/interface/patient_file/report/custom_report.php
index 156199f3b3..59cc446f17 100644
--- a/interface/patient_file/report/custom_report.php
+++ b/interface/patient_file/report/custom_report.php
@@ -32,10 +32,14 @@ use Mpdf\Mpdf;
 use OpenEMR\Common\Acl\AclMain;
 use OpenEMR\Core\Header;
 use OpenEMR\MedicalDevice\MedicalDevice;
 use OpenEMR\Services\FacilityService;
 
+if (!AclMain::aclCheckCore('patients', 'pat_rep')) {
+    die(xlt('Not authorized'));
+}
+
 $facilityService = new FacilityService();
 
 $staged_docs = array();
 $archive_name = '';
 
diff --git a/library/classes/Installer.class.php b/library/classes/Installer.class.php
index b5ef09d391..b150db5fc5 100644
--- a/library/classes/Installer.class.php
+++ b/library/classes/Installer.class.php
@@ -973,11 +973,11 @@ $config = 1; /////////////
 
         // Set permissions for front office staff.
         //
         $gacl->add_acl(
             array(
-                'patients' => array('alert','pat_rep')
+                'patients' => array('alert')
             ),
             null,
             array($front),
             null,
             null,
@@ -1015,11 +1015,11 @@ $config = 1; /////////////
             'Things that front office can read and partly modify'
         );
         // xl('Things that front office can read and partly modify')
         $gacl->add_acl(
             array(
-                'patients' => array('appt', 'demo', 'trans', 'notes'),
+                'patients' => array('appt', 'demo'),
                 'groups' => array('gcalendar')
             ),
             null,
             array($front),
             null,
@@ -1033,11 +1033,11 @@ $config = 1; /////////////
 
         // Set permissions for back office staff.
         //
         $gacl->add_acl(
             array(
-                'patients' => array('alert','pat_rep')
+                'patients' => array('alert')
             ),
             null,
             array($back),
             null,
             null,
diff --git a/src/Common/Acl/AclExtended.php b/src/Common/Acl/AclExtended.php
index 71143e823a..e94a0f8aa9 100644
--- a/src/Common/Acl/AclExtended.php
+++ b/src/Common/Acl/AclExtended.php
@@ -1017,10 +1017,47 @@ class AclExtended
         }
 
         return;
     }
 
+
+    /**
+     * Shift the ACL, opposite of updateAcl()
+     * Tries to remove the object from a specific ACL if only one is found.
+     *
+     * @param  array   $array_acl_id_number   Array containing hopefully one element, which is an integer, and is identifier of acl to be updated.
+     * @param  string  $group_title           Title of group.
+     * @param  string  $object_section_name   Identifier of section
+     * @param  string  $object_section_title  Title of section
+     * @param  string  $object_name           Identifier of object
+     * @param  string  $object_title          Title of object
+     * @param  string  $acl_return_value      What the acl returns (string), usually 'write', 'addonly', 'wsome' or 'view'
+     */
+    public static function shiftAcl($array_acl_id_number, $group_title, $section_name, $section_title, $object_name, $object_title, $return_value)
+    {
+        $gacl = self::collectGaclApiObject();
+        $tmp_array = $gacl->search_acl($section_name, $object_name, false, false, $group_title, false, false, false, $return_value);
+        switch (count($tmp_array)) {
+            case 0:
+                echo "The '$object_title' object of the '$section_title' section is not found in the '$group_title' group '$return_value' ACL.</BR>";
+                break;
+            case 1:
+                $tmp_boolean = @$gacl->shift_acl($array_acl_id_number[0], null, null, null, null, array($section_name => array($object_name)));
+                if ($tmp_boolean) {
+                    echo "Successfully removed the '$object_title' object of the '$section_title' section into the '$group_title' group '$return_value' ACL.</BR>";
+                } else {
+                    echo "<B>ERROR</B>,unable to remove the '$object_title' object of the '$section_title' section into the '$group_title' group '$return_value' ACL.</BR>";
+                }
+                break;
+            default:
+                echo "<B>ERROR</B>, Multiple '$group_title' group '$return_value' ACLs with the '$object_title' object of the '$section_title' section are present.</BR>";
+                break;
+        }
+
+        return;
+    }
+
     /**
      * Update the provided array of ACOs that the designated group has permission for.
      * This is an array keyed on ACO section ID with values that are arrays keyed on ACO ID
      * with values that are arrays keyed on return value.
      *
diff --git a/src/Gacl/GaclApi.php b/src/Gacl/GaclApi.php
index a421a93ed8..29c7c2f6cb 100644
--- a/src/Gacl/GaclApi.php
+++ b/src/Gacl/GaclApi.php
@@ -402,12 +402,13 @@ class GaclApi extends Gacl {
 		if (empty($acl_id)) {
 			$this->debug_text("append_acl(): No ACL_ID specified! ACL_ID: $acl_id");
 			return false;
 		}
 
-		//Grab ACL data.
-		$acl_array = &$this->get_acl($acl_id);
+        //Grab ACL data.
+        $get_acl = $this->get_acl($acl_id);
+        $acl_array = &$get_acl;
 
 		//Append each object type seperately.
 		if (is_array($aro_array) AND count($aro_array) > 0) {
 			$this->debug_text("append_acl(): Appending ARO's");
 
@@ -522,12 +523,13 @@ class GaclApi extends Gacl {
 		if (empty($acl_id)) {
 			$this->debug_text("shift_acl(): No ACL_ID specified! ACL_ID: $acl_id");
 			return false;
 		}
 
-		//Grab ACL data.
-		$acl_array = &$this->get_acl($acl_id);
+        //Grab ACL data.
+        $get_acl = $this->get_acl($acl_id);
+        $acl_array = &$get_acl;
 
 		//showarray($acl_array);
 		//Remove each object type seperately.
 		if (is_array($aro_array) AND count($aro_array) > 0) {
 			$this->debug_text("shift_acl(): Removing ARO's");
@@ -610,11 +612,11 @@ class GaclApi extends Gacl {
 			$this->debug_text("shift_acl(): Removing ACO's");
 
 			foreach ($aco_array as $aco_section_value => $aco_value_array) {
 				foreach ($aco_value_array as $aco_value) {
 					$this->debug_text("shift_acl(): ACO Section Value: $aco_section_value ACO VALUE: $aco_value");
-					$aco_key = array_search($aco_value, $acl_array['aco'][$aco_section_value]);
+					$aco_key = array_search($aco_value, ($acl_array['aco'][$aco_section_value] ?? []));
 
 					if ($aco_key !== FALSE) {
 						$this->debug_text("shift_acl(): Removing ACO. ($aco_key)");
 						unset($acl_array['aco'][$aco_section_value][$aco_key]);
 						$update = 1;
diff --git a/version.php b/version.php
index ecc129e8ae..414a08b8fe 100644
--- a/version.php
+++ b/version.php
@@ -34,11 +34,11 @@ $v_database = 435;
 // is a access control change in the course of development.  It is used
 // during installation to determine what the installed version of the access
 // controls is (subsequently the acl_upgrade.php script then is used to
 // upgrade and track this value)
 //
-$v_acl = 11;
+$v_acl = 12;
 
 // Version for JavaScript and stylesheet includes. Increment whenever a .js or .css file changes.
 // Also whenever you change a .js or .css file, make sure that all URLs referencing it
 // end with "?v=$v_js_includes".  Search the code for examples of doing this.
 // All this is to keep browsers from using an older cached version.

Vulnerable code: 
/* This is a template for a new revision, when needed
                'patients' => array('alert','pat_rep')
                'patients' => array('appt', 'demo', 'trans', 'notes'),
                'patients' => array('alert','pat_rep')
		//Grab ACL data.
		$acl_array = &$this->get_acl($acl_id);
		//Grab ACL data.
		$acl_array = &$this->get_acl($acl_id);
					$aco_key = array_search($aco_value, $acl_array['aco'][$aco_section_value]);
$v_acl = 11;
Fixed code:     //Collect the ACL ID numbers.
    echo "<B>Checking to ensure all the proper ACL(access control list) are present:</B></BR>";
    $accounting_view = AclExtended::getAclIdNumber('Accounting', 'view');
    $frontoffice_view = AclExtended::getAclIdNumber('Front Office', 'view');
    $frontoffice_write = AclExtended::getAclIdNumber('Front Office', 'write');

    //Add new object Sections
    echo "<BR/><B>Adding new object sections</B><BR/>";

    //Add new Objects
    echo "<BR/><B>Adding new objects</B><BR/>";

    //Update already existing Objects
    echo "<BR/><B>Upgrading objects</B><BR/>";

    //Add new ACLs here (will return the ACL ID of newly created or already existant ACL)
    // (will also place in the appropriate group and CREATE a new group if needed)
    echo "<BR/><B>Adding ACLs(Access Control Lists) and groups</B><BR/>";

    //Update the ACLs
    echo "<BR/><B>Updating the ACLs(Access Control Lists)</B><BR/>";
    AclExtended::shiftAcl($accounting_view, 'Accounting', 'patients', 'Patients', 'pat_rep', 'Patient Report', 'view');
    AclExtended::shiftAcl($frontoffice_view, 'Front Office', 'patients', 'Patients', 'pat_rep', 'Patient Report', 'view');
    AclExtended::shiftAcl($frontoffice_write, 'Front Office', 'patients', 'Patients', 'trans', 'Transactions (write,wsome optional)', 'write');
    AclExtended::shiftAcl($frontoffice_write, 'Front Office', 'patients', 'Patients', 'notes', 'Patient Notes (write,wsome optional)', 'write');


    //DONE with upgrading to this version
    $acl_version = $upgrade_acl;
}

/* This is a template for a new revision, when needed
// Upgrade for acl_version 13
$upgrade_acl = 13;
if ($acl_version < $upgrade_acl) {
    echo "<B>UPGRADING ACCESS CONTROLS TO VERSION " . $upgrade_acl . ":</B></BR>";

if (!AclMain::aclCheckCore('patients', 'pat_rep')) {
    die(xlt('Not authorized'));
}

                'patients' => array('alert')
                'patients' => array('appt', 'demo'),
                'patients' => array('alert')

    /**
     * Shift the ACL, opposite of updateAcl()
     * Tries to remove the object from a specific ACL if only one is found.
     *
     * @param  array   $array_acl_id_number   Array containing hopefully one element, which is an integer, and is identifier of acl to be updated.
     * @param  string  $group_title           Title of group.
     * @param  string  $object_section_name   Identifier of section
     * @param  string  $object_section_title  Title of section
     * @param  string  $object_name           Identifier of object
     * @param  string  $object_title          Title of object
     * @param  string  $acl_return_value      What the acl returns (string), usually 'write', 'addonly', 'wsome' or 'view'
     */
    public static function shiftAcl($array_acl_id_number, $group_title, $section_name, $section_title, $object_name, $object_title, $return_value)
    {
        $gacl = self::collectGaclApiObject();
        $tmp_array = $gacl->search_acl($section_name, $object_name, false, false, $group_title, false, false, false, $return_value);
        switch (count($tmp_array)) {
            case 0:
                echo "The '$object_title' object of the '$section_title' section is not found in the '$group_title' group '$return_value' ACL.</BR>";
                break;
            case 1:
                $tmp_boolean = @$gacl->shift_acl($array_acl_id_number[0], null, null, null, null, array($section_name => array($object_name)));
                if ($tmp_boolean) {
                    echo "Successfully removed the '$object_title' object of the '$section_title' section into the '$group_title' group '$return_value' ACL.</BR>";
                } else {
                    echo "<B>ERROR</B>,unable to remove the '$object_title' object of the '$section_title' section into the '$group_title' group '$return_value' ACL.</BR>";
                }
                break;
            default:
                echo "<B>ERROR</B>, Multiple '$group_title' group '$return_value' ACLs with the '$object_title' object of the '$section_title' section are present.</BR>";
                break;
        }

        return;
    }

        //Grab ACL data.
        $get_acl = $this->get_acl($acl_id);
        $acl_array = &$get_acl;
        //Grab ACL data.
        $get_acl = $this->get_acl($acl_id);
        $acl_array = &$get_acl;
					$aco_key = array_search($aco_value, ($acl_array['aco'][$aco_section_value] ?? []));
$v_acl = 12;