CVE-2023-3303: Improper Access Control (CWE-284)
value: Improper Access Control in GitHub repository admidio/admidio prior to 4.2.9.
CWE description: The product does not restrict or incorrectly restricts access to a resource from an unauthorized actor.
CVSS v3 base score: 3.5
Published: 2023-06-23T13:15Z
Repository: https://github.com/admidio/admidio
Language: PHP
Files changed: adm_program/modules/ecards/ecard_send.php, adm_program/modules/ecards/ecards.php, adm_program/modules/photos/photos.php, adm_program/system/classes/TablePhotos.php
Commit date: 2023-06-08 07:21:41 +0200
Commit message: ecard could sent if album is logged #1432

Diff: diff --git a/adm_program/modules/ecards/ecard_send.php b/adm_program/modules/ecards/ecard_send.php
index e674f7983..ce829c7c0 100644
--- a/adm_program/modules/ecards/ecard_send.php
+++ b/adm_program/modules/ecards/ecard_send.php
@@ -9,10 +9,16 @@
  ***********************************************************************************************
  */
 require_once(__DIR__ . '/../../system/common.php');
 require_once(__DIR__ . '/ecard_function.php');
 
+// check if the module is enabled and disallow access if it's disabled
+if (!$gSettingsManager->getBool('enable_ecard_module')) {
+    $gMessage->show($gL10n->get('SYS_MODULE_DISABLED'));
+    // => EXIT
+}
+
 // Initialize and check the parameters
 $postTemplateName = admFuncVariableIsValid($_POST, 'ecard_template', 'file', array('requireValue' => true));
 $postPhotoUuid    = admFuncVariableIsValid($_POST, 'photo_uuid', 'string', array('requireValue' => true));
 $postPhotoNr      = admFuncVariableIsValid($_POST, 'photo_nr', 'int', array('requireValue' => true));
 $postMessage      = admFuncVariableIsValid($_POST, 'ecard_message', 'html');
@@ -31,18 +37,19 @@ try {
 } catch (AdmException $exception) {
     $exception->showHtml();
     // => EXIT
 }
 
-// check if the module is enabled and disallow access if it's disabled
-if (!$gSettingsManager->getBool('enable_ecard_module')) {
-    $gMessage->show($gL10n->get('SYS_MODULE_DISABLED'));
+// check if user has right to view the album
+if (!$photoAlbum->isVisible()) {
+    $gMessage->show($gL10n->get('SYS_INVALID_PAGE_VIEW'));
     // => EXIT
 }
-// pruefen ob User eingeloggt ist
-if (!$gValidLogin) {
-    $gMessage->show($gL10n->get('SYS_INVALID_PAGE_VIEW'));
+
+// the logged-in user has no valid mail address stored in his profile, which can be used as sender
+if ($gValidLogin && $gCurrentUser->getValue('EMAIL') === '') {
+    $gMessage->show($gL10n->get('SYS_CURRENT_USER_NO_EMAIL', array('<a href="'.ADMIDIO_URL.FOLDER_MODULES.'/profile/profile.php">', '</a>')));
     // => EXIT
 }
 
 $senderName  = $gCurrentUser->getValue('FIRST_NAME').' '.$gCurrentUser->getValue('LAST_NAME');
 $senderEmail = $gCurrentUser->getValue('EMAIL');
@@ -56,11 +63,11 @@ if (!isset($_POST['ecard_recipients']) || !is_array($_POST['ecard_recipients']))
 if ($postMessage === '') {
     $gMessage->show($gL10n->get('SYS_FIELD_EMPTY', array($gL10n->get('SYS_MESSAGE'))));
     // => EXIT
 }
 
-// Template wird geholt
+// read template from file system
 $ecardDataToParse = $funcClass->getEcardTemplate($postTemplateName);
 
 // if template was not found then show error
 if ($ecardDataToParse === null) {
     $gMessage->show($gL10n->get('SYS_MODULE_DISABLED'));
diff --git a/adm_program/modules/ecards/ecards.php b/adm_program/modules/ecards/ecards.php
index 0f6c12d02..03f00e759 100644
--- a/adm_program/modules/ecards/ecards.php
+++ b/adm_program/modules/ecards/ecards.php
@@ -17,11 +17,11 @@
 require_once(__DIR__ . '/../../system/common.php');
 require_once(__DIR__ . '/ecard_function.php');
 require(__DIR__ . '/../../system/login_valid.php');
 
 // Initialize and check the parameters
-$getPhotoUuid = admFuncVariableIsValid($_GET, 'photo_uuid', 'string');
+$getPhotoUuid = admFuncVariableIsValid($_GET, 'photo_uuid', 'string', array('requireValue' => true));
 $getUserUuid  = admFuncVariableIsValid($_GET, 'user_uuid', 'string');
 $getPhotoNr   = admFuncVariableIsValid($_GET, 'photo_nr', 'int', array('requireValue' => true));
 $showPage     = admFuncVariableIsValid($_GET, 'show_page', 'int', array('defaultValue' => 1));
 
 // Initialisierung lokaler Variablen
@@ -33,51 +33,47 @@ $headline  = $gL10n->get('SYS_SEND_GREETING_CARD');
 if (!$gSettingsManager->getBool('enable_ecard_module')) {
     $gMessage->show($gL10n->get('SYS_MODULE_DISABLED'));
     // => EXIT
 }
 
-// URL auf Navigationstack ablegen
+// Drop URL on navigation stack
 $gNavigation->addUrl(CURRENT_URL, $headline);
 
-// Fotoveranstaltungs-Objekt erzeugen oder aus Session lesen
+// Create photo album object or read from session
 if (isset($_SESSION['photo_album']) && (int) $_SESSION['photo_album']->getValue('pho_uuid') === $getPhotoUuid) {
     $photoAlbum =& $_SESSION['photo_album'];
 } else {
-    // einlesen des Albums falls noch nicht in Session gespeichert
     $photoAlbum = new TablePhotos($gDb);
-    if ($getPhotoUuid !== '') {
-        $photoAlbum->readDataByUuid($getPhotoUuid);
-    }
+    $photoAlbum->readDataByUuid($getPhotoUuid);
 
     $_SESSION['photo_album'] = $photoAlbum;
 }
 
-// pruefen, ob Album zur aktuellen Organisation gehoert
-if ($getPhotoUuid !== '' && (int) $photoAlbum->getValue('pho_org_id') !== $gCurrentOrgId) {
+// check if user has right to view the album
+if (!$photoAlbum->isVisible()) {
     $gMessage->show($gL10n->get('SYS_INVALID_PAGE_VIEW'));
     // => EXIT
 }
 
 if ($gValidLogin && $gCurrentUser->getValue('EMAIL') === '') {
-    // der eingeloggte Benutzer hat in seinem Profil keine gueltige Mailadresse hinterlegt,
-    // die als Absender genutzt werden kann...
+    // the logged in user has no valid mail address stored in his profile, which can be used as sender
     $gMessage->show($gL10n->get('SYS_CURRENT_USER_NO_EMAIL', array('<a href="'.ADMIDIO_URL.FOLDER_MODULES.'/profile/profile.php">', '</a>')));
     // => EXIT
 }
 
 if ($getUserUuid !== '') {
-    // usr_id wurde uebergeben, dann Kontaktdaten des Users aus der DB fischen
+    // UUID was set than read contact data of this user
     $user = new User($gDb, $gProfileFields);
     $user->readDataByUuid($getUserUuid);
 
-    // darf auf die User-Id zugegriffen werden
+    // check if the current user has the right communicate with that member
     if ((!$gCurrentUser->editUsers() && !isMember((int) $user->getValue('usr_id'))) || strlen($user->getValue('usr_id')) === 0) {
         $gMessage->show($gL10n->get('SYS_USER_ID_NOT_FOUND'));
         // => EXIT
     }
 
-    // besitzt der User eine gueltige E-Mail-Adresse
+    // check if the member has a valid email address
     if (!StringUtils::strValidCharacters($user->getValue('EMAIL'), 'email')) {
         $gMessage->show($gL10n->get('SYS_USER_NO_EMAIL', array($user->getValue('FIRST_NAME').' '.$user->getValue('LAST_NAME'))));
         // => EXIT
     }
 }
diff --git a/adm_program/modules/photos/photos.php b/adm_program/modules/photos/photos.php
index bf8f4f6f6..00f3d57c1 100644
--- a/adm_program/modules/photos/photos.php
+++ b/adm_program/modules/photos/photos.php
@@ -57,11 +57,11 @@ if ($getPhotoUuid !== '') {
         // => EXIT
     }
 
     $headline = $photoAlbum->getValue('pho_name');
 
-    // URL auf Navigationstack ablegen
+    // Drop URL on navigation stack
     $gNavigation->addUrl(CURRENT_URL, $headline);
 } else {
     $headline = $getHeadline;
 
     // Navigation of the module starts here
diff --git a/adm_program/system/classes/TablePhotos.php b/adm_program/system/classes/TablePhotos.php
index 2ede7e9b8..36384caf6 100644
--- a/adm_program/system/classes/TablePhotos.php
+++ b/adm_program/system/classes/TablePhotos.php
@@ -230,11 +230,11 @@ class TablePhotos extends TableAccess
         // current photo album must belong to current organization
         if ($this->getValue('pho_id') > 0 && (int) $this->getValue('pho_org_id') !== $GLOBALS['gCurrentOrgId']) {
             return false;
         }
         // locked photo album could only be viewed by module administrators
-        elseif ((int) $this->getValue('pho_locked') === 1 && !$GLOBALS['gCurrentUser']->editPhotoRight()) {
+        elseif ($this->getValue('pho_locked') && !$GLOBALS['gCurrentUser']->editPhotoRight()) {
             return false;
         }
 
         return true;
     }

Vulnerable code: // check if the module is enabled and disallow access if it's disabled
if (!$gSettingsManager->getBool('enable_ecard_module')) {
    $gMessage->show($gL10n->get('SYS_MODULE_DISABLED'));
// pruefen ob User eingeloggt ist
if (!$gValidLogin) {
    $gMessage->show($gL10n->get('SYS_INVALID_PAGE_VIEW'));
// Template wird geholt
$getPhotoUuid = admFuncVariableIsValid($_GET, 'photo_uuid', 'string');
// URL auf Navigationstack ablegen
// Fotoveranstaltungs-Objekt erzeugen oder aus Session lesen
    // einlesen des Albums falls noch nicht in Session gespeichert
    if ($getPhotoUuid !== '') {
        $photoAlbum->readDataByUuid($getPhotoUuid);
    }
// pruefen, ob Album zur aktuellen Organisation gehoert
if ($getPhotoUuid !== '' && (int) $photoAlbum->getValue('pho_org_id') !== $gCurrentOrgId) {
    // der eingeloggte Benutzer hat in seinem Profil keine gueltige Mailadresse hinterlegt,
    // die als Absender genutzt werden kann...
    // usr_id wurde uebergeben, dann Kontaktdaten des Users aus der DB fischen
    // darf auf die User-Id zugegriffen werden
    // besitzt der User eine gueltige E-Mail-Adresse
    // URL auf Navigationstack ablegen
        elseif ((int) $this->getValue('pho_locked') === 1 && !$GLOBALS['gCurrentUser']->editPhotoRight()) {
Fixed code: // check if the module is enabled and disallow access if it's disabled
if (!$gSettingsManager->getBool('enable_ecard_module')) {
    $gMessage->show($gL10n->get('SYS_MODULE_DISABLED'));
    // => EXIT
}

// check if user has right to view the album
if (!$photoAlbum->isVisible()) {
    $gMessage->show($gL10n->get('SYS_INVALID_PAGE_VIEW'));

// the logged-in user has no valid mail address stored in his profile, which can be used as sender
if ($gValidLogin && $gCurrentUser->getValue('EMAIL') === '') {
    $gMessage->show($gL10n->get('SYS_CURRENT_USER_NO_EMAIL', array('<a href="'.ADMIDIO_URL.FOLDER_MODULES.'/profile/profile.php">', '</a>')));
// read template from file system
$getPhotoUuid = admFuncVariableIsValid($_GET, 'photo_uuid', 'string', array('requireValue' => true));
// Drop URL on navigation stack
// Create photo album object or read from session
    $photoAlbum->readDataByUuid($getPhotoUuid);
// check if user has right to view the album
if (!$photoAlbum->isVisible()) {
    // the logged in user has no valid mail address stored in his profile, which can be used as sender
    // UUID was set than read contact data of this user
    // check if the current user has the right communicate with that member
    // check if the member has a valid email address
    // Drop URL on navigation stack
        elseif ($this->getValue('pho_locked') && !$GLOBALS['gCurrentUser']->editPhotoRight()) {