Fix sqli vuln (#7086)

# Description & Issue number it closes addresses sql injection concern and added additional type safety
ChurchCRM · Jul 2, 2024 · e3bd7bf · e3bd7bf
2 parents a16602b + 1ed1f2e
commit e3bd7bf
Show file tree

Hide file tree

Showing 4 changed files with 49 additions and 39 deletions.
diff --git a/src/AddDonors.php b/src/AddDonors.php
@@ -10,17 +10,13 @@
 if (array_key_exists('linkBack', $_GET)) {
     InputUtils::legacyFilterInput($_GET['linkBack']);
 }
-$iFundRaiserID = InputUtils::legacyFilterInput($_GET['FundRaiserID']);
+$iFundRaiserID = InputUtils::filterInt($_GET['FundRaiserID']);
 
 if ($linkBack === '') {
     $linkBack = "PaddleNumList.php?FundRaiserID=$iFundRaiserID";
 }
 
 if ($iFundRaiserID > 0) {
-    // Get the current fund raiser record
-    $sSQL = 'SELECT * from fundraiser_fr WHERE fr_ID = ' . $iFundRaiserID;
-    $rsFRR = RunQuery($sSQL);
-    extract(mysqli_fetch_array($rsFRR));
     // Set current fundraiser
     $_SESSION['iCurrentFundraiser'] = $iFundRaiserID;
 } else {

diff --git a/src/GetText.php b/src/GetText.php
@@ -1,19 +1,26 @@
 <?php
 
+use ChurchCRM\model\ChurchCRM\EventQuery;
+use ChurchCRM\Utils\InputUtils;
+use ChurchCRM\Utils\LoggerUtils;
+
 require 'Include/Config.php';
 require 'Include/Functions.php';
 
-$sSQL = 'SELECT * FROM events_event WHERE event_id = ' . $_GET['EID'];
-$rsOpps = RunQuery($sSQL);
-$aRow = mysqli_fetch_array($rsOpps, MYSQLI_BOTH) || die(mysqli_error($cnInfoCentral));
-extract($aRow);
-$aEventID = $event_id;
-$aEventTitle = $event_title;
-$aEventText = $event_text;
+$eidQueryParam = $_GET['EID'];
+$sanitizedEidQueryParam = InputUtils::filterInt($eidQueryParam);
+if ($eidQueryParam !== (string) $sanitizedEidQueryParam) {
+    LoggerUtils::getAppLogger()->warning('Provided event ID does not match sanitized event ID', ['providedEventId' => $eidQueryParam, 'sanitizedEventId' => $sanitizedEidQueryParam]);
+}
+
+$event = EventQuery::create()->findOneById($sanitizedEidQueryParam);
+$aEventID = $event->getId();
+$aEventTitle = $event->getTitle();
+$aEventText = $event->getText();
 ?>
 <html>
 <head><title><?= gettext("Text from") ?> <?= $aEventID ?></title></head>
-</html>
+<body>
 <table cellpadding="4" align="center" cellspacing="0" width="100%">
   <caption>
     <h3><?= gettext('Text for Event ID: ') . $aEventTitle ?></h3>
@@ -26,4 +33,6 @@
       <input type="button" name="Action" value="Close Window" class="btn btn-default" onclick="javascript:window.close()">
     </td>
   </tr>
+</table>
+</body>
 </html>
diff --git a/src/VolunteerOpportunityEditor.php b/src/VolunteerOpportunityEditor.php
@@ -35,15 +35,15 @@
     $sAction = InputUtils::legacyFilterInput($_GET['act']);
 }
 if (array_key_exists('Opp', $_GET)) {
-    $iOpp = InputUtils::legacyFilterInput($_GET['Opp'], 'int');
+    $iOpp = InputUtils::filterInt($_GET['Opp']);
 }
 if (array_key_exists('row_num', $_GET)) {
-    $iRowNum = InputUtils::legacyFilterInput($_GET['row_num'], 'int');
+    $iRowNum = InputUtils::filterInt($_GET['row_num']);
 }
 
 $sDeleteError = '';
 
-if (($sAction == 'delete') && $iOpp > 0) {
+if ($sAction === 'delete' && $iOpp > 0) {
     // Delete Confirmation Page
 
     // Security: User must have Delete records permission
@@ -105,7 +105,7 @@
     exit;
 }
 
-if (($sAction == 'ConfDelete') && $iOpp > 0) {
+if ($sAction === 'ConfDelete' && $iOpp > 0) {
     // Security: User must have Delete records permission
     // Otherwise, redirect to the main menu
     AuthenticationManager::redirectHomeIfFalse(AuthenticationManager::getCurrentUser()->isDeleteRecordsEnabled());
@@ -124,7 +124,7 @@
     RunQuery($sSQL);
 }
 
-if ($iRowNum == 0) {
+if ($iRowNum === 0) {
     // Skip data integrity check if we are only changing the ordering
     // by moving items up or down.
     // System response is too slow to do these checks every time the page
@@ -192,7 +192,7 @@
         if (array_key_exists($nameName, $_POST)) {
             $aNameFields[$iFieldID] = InputUtils::legacyFilterInput($_POST[$nameName]);
 
-            if (strlen($aNameFields[$iFieldID]) == 0) {
+            if (strlen($aNameFields[$iFieldID]) === 0) {
                 $aNameErrors[$iFieldID] = true;
                 $bErrorFlag = true;
             } else {
@@ -222,7 +222,7 @@
     if (isset($_POST['AddField'])) { // Check if we're adding a VolOp
         $newFieldName = InputUtils::legacyFilterInput($_POST['newFieldName']);
         $newFieldDesc = InputUtils::legacyFilterInput($_POST['newFieldDesc']);
-        if (strlen($newFieldName) == 0) {
+        if (strlen($newFieldName) === 0) {
             $bNewNameError = true;
         } else { // Insert into table
             // There must be an easier way to get the number of rows in order to generate the last order number.
@@ -274,9 +274,9 @@
                     if ($iRowNum && $sAction != '') {
                         // cast as int and couple with switch for sql injection prevention for $row_num
                         $swapRow = $iRowNum;
-                        if ($sAction == 'up') {
+                        if ($sAction === 'up') {
                             $newRow = --$iRowNum;
-                        } elseif ($sAction == 'down') {
+                        } elseif ($sAction === 'down') {
                             $newRow = ++$iRowNum;
                         } else {
                             $newRow = $iRowNum;
@@ -346,7 +346,7 @@
                         echo '<tr>';
                         echo '<td class="LabelColumn"><b>' . $row . '</b></td>';
                         echo '<td class="TextColumn">';
-                        if ($row == 1) {
+                        if ($row === 1) {
                             echo '<a href="VolunteerOpportunityEditor.php?act=na&amp;row_num=' . $row . "\"><i class='fa fa-fw'></i></a>";
                         } else {
                             echo '<a href="VolunteerOpportunityEditor.php?act=up&amp;row_num=' . $row . "\"> <i class='fa fa-arrow-up'></i></a> ";

diff --git a/src/WhyCameEditor.php b/src/WhyCameEditor.php
@@ -3,18 +3,22 @@
 require 'Include/Config.php';
 require 'Include/Functions.php';
 
+use ChurchCRM\model\ChurchCRM\PersonQuery;
 use ChurchCRM\model\ChurchCRM\WhyCame;
 use ChurchCRM\model\ChurchCRM\WhyCameQuery;
 use ChurchCRM\Utils\InputUtils;
+use ChurchCRM\Utils\LoggerUtils;
 use ChurchCRM\Utils\RedirectUtils;
 
+$logger = LoggerUtils::getAppLogger();
+
 $linkBack = InputUtils::legacyFilterInput($_GET['linkBack']);
-$iPerson = InputUtils::legacyFilterInput($_GET['PersonID']);
-$iWhyCameID = InputUtils::legacyFilterInput($_GET['WhyCameID']);
+$iPerson = InputUtils::filterInt($_GET['PersonID']);
+$iWhyCameID = InputUtils::filterInt($_GET['WhyCameID']);
 
-$sSQL = 'SELECT per_FirstName, per_LastName FROM person_per where per_ID = ' . $iPerson;
-$rsPerson = RunQuery($sSQL);
-extract(mysqli_fetch_array($rsPerson));
+$person = PersonQuery::create()->findOneById($iPerson);
+$per_FirstName = $person->getFirstName();
+$per_LastName = $person->getLastName();
 
 $sPageTitle = gettext('"Why Came" notes for ') . $per_FirstName . ' ' . $per_LastName;
 
@@ -56,17 +60,18 @@
         }
     }
 } else {
-    $sSQL = 'SELECT * FROM whycame_why WHERE why_per_ID = ' . $iPerson;
-    $rsWhyCame = RunQuery($sSQL);
-    if (mysqli_num_rows($rsWhyCame) > 0) {
-        extract(mysqli_fetch_array($rsWhyCame));
+    $whyCames = WhyCameQuery::create()->findByPerId($iPerson);
+    if (count($whyCames) > 0) {
+        if (count($whyCames) > 1) {
+            $logger->warning('multiple why came records found for person', ['personId' => $iPerson]);
+        }
+        $whyCame = $whyCames[0];
 
-        $iWhyCameID = $why_ID;
-        $tJoin = $why_join;
-        $tCome = $why_come;
-        $tSuggest = $why_suggest;
-        $tHearOfUs = $why_hearOfUs;
-    } else {
+        $iWhyCameID = $whyCame->getId();
+        $tJoin = $whyCame->getJoin();
+        $tCome = $whyCame->getCome();
+        $tSuggest = $whyCame->getSuggest();
+        $tHearOfUs = $whyCame->getHearOfUs();
     }
 }
 
@@ -104,8 +109,8 @@
                                                                 } ?>';">
           </td>
         </tr>
+      </table>
     </form>
-    </table>
   </div>
 </div>
 <?php