[security] Fixed issue #12436: potential SQL injection through Notifi…

…cations Dev: ALLWAYS USE PDO OR Yii::app()->db->quoteValue
LimeSurvey · May 31, 2017 · 8ed8724 · 8ed8724
1 parent 9e6f722
commit 8ed8724
Showing 1 changed file with 11 additions and 13 deletions.
diff --git a/application/models/Notification.php b/application/models/Notification.php
@@ -368,30 +368,28 @@ public static function countImportantNotifications($surveyId)
     protected static function getCriteria($surveyId = null)
     {
         $criteria = new CDbCriteria();
-
+        $params = array();
         // Only fetch survey specific notifications if user is viewing a survey
         if (!empty($surveyId))
         {
-            $criteria->addCondition('entity = \'survey\'');
-            $criteria->addCondition('entity_id = ' . $surveyId);  // TODO: Escape survey id
+            $criteria->addCondition('entity =:sentity AND entity_id=:sentity_id');
+            $params[':sentity'] = 'survey';
+            $params[':sentity_id'] = $surveyId;
         }
-
         // User notifications
-        $criteria2 = new CDbCriteria();
-        $criteria2->addCondition('entity = \'user\'');
-        $criteria2->addCondition('entity_id = ' . Yii::app()->user->id);  // TODO: Escape
+        $criteria->addCondition('entity =:uentity AND entity_id=:uentity_id','OR');
+        $params[':uentity'] = 'user';
+        $params[':uentity_id'] = Yii::app()->user->id;
 
         // Only get new notifications
         //$criteria3 = new CDbCriteria();
         //$criteria3->addCondition('status = \'new\'');  // TODO: read = null?
-
-        $criteria->mergeWith($criteria2, 'OR');
         //$criteria->mergeWith($criteria3, 'AND');
-        $criteria->mergeWith(array(
-            'order' => 'id DESC',
-            'limit' => 50
-        ));
 
+        $criteria->params=$params;
+        $criteria->order = 'id DESC';
+        $criteria->limit = 50;
+
         return $criteria;
     }