From 21d32567e1b27f08ea060ed31199656365b15b7e Mon Sep 17 00:00:00 2001 From: "Donald A. Lobo" Date: Tue, 8 Oct 2013 15:53:59 +0100 Subject: [PATCH] CRM-13554 - validate values of order by ---------------------------------------- * CRM-13554: Improve string validation in the query engine http://issues.civicrm.org/jira/browse/CRM-13554 --- CRM/Activity/BAO/Activity.php | 3 ++- CRM/Batch/BAO/Batch.php | 6 ++--- CRM/Case/BAO/Case.php | 4 ++- CRM/Contact/BAO/Group.php | 4 +-- .../Form/Search/Custom/ActivitySearch.php | 1 + CRM/Contact/Form/Search/Custom/Base.php | 1 + .../Form/Search/Custom/EmployerListing.php | 1 + CRM/Contact/Form/Search/Custom/Group.php | 1 + .../Form/Search/Custom/TagContributions.php | 1 + CRM/Core/BAO/FinancialTrxn.php | 25 +++++++++++-------- CRM/Mailing/Event/BAO/Bounce.php | 1 + CRM/Mailing/Event/BAO/Delivered.php | 1 + CRM/Mailing/Event/BAO/Forward.php | 1 + CRM/Mailing/Event/BAO/Opened.php | 5 ++-- CRM/Mailing/Event/BAO/Queue.php | 9 ++++--- CRM/Mailing/Event/BAO/Reply.php | 1 + CRM/Mailing/Event/BAO/TrackableURLOpen.php | 1 + CRM/Mailing/Event/BAO/Unsubscribe.php | 1 + 18 files changed, 44 insertions(+), 23 deletions(-) diff --git a/CRM/Activity/BAO/Activity.php b/CRM/Activity/BAO/Activity.php index d28b76373d..99c4abfbd6 100644 --- a/CRM/Activity/BAO/Activity.php +++ b/CRM/Activity/BAO/Activity.php @@ -738,7 +738,8 @@ class CRM_Activity_BAO_Activity extends CRM_Activity_DAO_Activity { } } elseif (trim($input['sort'])) { - $order = " ORDER BY {$input['sort']}"; + $sort = CRM_Utils_Type::escape($input['sort'], 'String'); + $order = " ORDER BY $sort "; } } diff --git a/CRM/Batch/BAO/Batch.php b/CRM/Batch/BAO/Batch.php index d203cbd134..62a86667cb 100644 --- a/CRM/Batch/BAO/Batch.php +++ b/CRM/Batch/BAO/Batch.php @@ -266,7 +266,7 @@ class CRM_Batch_BAO_Batch extends CRM_Batch_DAO_Batch { $orderBy = ' ORDER BY batch.id desc'; if (!empty($params['sort'])) { - $orderBy = ' ORDER BY ' . $params['sort']; + $orderBy = ' ORDER BY ' . CRM_Utils_Type::escape($params['sort'], 'String'); } $query = " @@ -633,8 +633,8 @@ class CRM_Batch_BAO_Batch extends CRM_Batch_DAO_Batch { } $orderBy = " ORDER BY civicrm_financial_trxn.id"; - if (CRM_Utils_Array::value('sort', $params)) { - $orderBy = ' ORDER BY ' . CRM_Utils_Array::value('sort', $params); + if (!empty($params['sort'])) { + $orderBy = ' ORDER BY ' . CRM_Utils_Type::escape($params['sort'], 'String'); } $from = "civicrm_financial_trxn diff --git a/CRM/Case/BAO/Case.php b/CRM/Case/BAO/Case.php index 05c534bca6..cc1c024110 100644 --- a/CRM/Case/BAO/Case.php +++ b/CRM/Case/BAO/Case.php @@ -1066,7 +1066,9 @@ SELECT case_status.label AS case_status, status_id, case_type.label AS case_type $orderBy = " ORDER BY overdue_date ASC, display_date DESC, weight DESC"; } else { - $orderBy = " ORDER BY {$sortname} {$sortorder}"; + $sort = "{$sortname} {$sortorder}"; + $sort = CRM_Utils_Type::escape($sort, 'String'); + $orderBy = " ORDER BY $sort "; if ($sortname != 'display_date') { $orderBy .= ', display_date DESC'; } diff --git a/CRM/Contact/BAO/Group.php b/CRM/Contact/BAO/Group.php index 5fd504daed..1007b8c6cd 100644 --- a/CRM/Contact/BAO/Group.php +++ b/CRM/Contact/BAO/Group.php @@ -728,8 +728,8 @@ class CRM_Contact_BAO_Group extends CRM_Contact_DAO_Group { } $orderBy = ' ORDER BY groups.title asc'; - if (CRM_Utils_Array::value('sort', $params)) { - $orderBy = ' ORDER BY ' . CRM_Utils_Array::value('sort', $params); + if (!empty($params['sort'])) { + $orderBy = ' ORDER BY ' . CRM_Utils_Type::escape($params['sort'], 'String'); } $select = $from = $where = ""; diff --git a/CRM/Contact/Form/Search/Custom/ActivitySearch.php b/CRM/Contact/Form/Search/Custom/ActivitySearch.php index 5113f49fa4..088480f3b9 100644 --- a/CRM/Contact/Form/Search/Custom/ActivitySearch.php +++ b/CRM/Contact/Form/Search/Custom/ActivitySearch.php @@ -201,6 +201,7 @@ class CRM_Contact_Form_Search_Custom_ActivitySearch implements CRM_Contact_Form_ // Define ORDER BY for query in $sort, with default value if (!empty($sort)) { if (is_string($sort)) { + $sort = CRM_Utils_Type::escape($sort, 'String'); $sql .= " ORDER BY $sort "; } else { diff --git a/CRM/Contact/Form/Search/Custom/Base.php b/CRM/Contact/Form/Search/Custom/Base.php index 781cfc9071..a7f5488393 100644 --- a/CRM/Contact/Form/Search/Custom/Base.php +++ b/CRM/Contact/Form/Search/Custom/Base.php @@ -124,6 +124,7 @@ class CRM_Contact_Form_Search_Custom_Base { function addSortOffset(&$sql, $offset, $rowcount, $sort) { if (!empty($sort)) { if (is_string($sort)) { + $sort = CRM_Utils_Type::escape($sort, 'String'); $sql .= " ORDER BY $sort "; } else { diff --git a/CRM/Contact/Form/Search/Custom/EmployerListing.php b/CRM/Contact/Form/Search/Custom/EmployerListing.php index 2b304afabb..681bc4e7f8 100644 --- a/CRM/Contact/Form/Search/Custom/EmployerListing.php +++ b/CRM/Contact/Form/Search/Custom/EmployerListing.php @@ -134,6 +134,7 @@ class CRM_Contact_Form_Search_Custom_EmployerListing implements CRM_Contact_Form // Define ORDER BY for query in $sort, with default value if (!empty($sort)) { if (is_string($sort)) { + $sort = CRM_Utils_Type::escape($sort, 'String'); $sql .= " ORDER BY $sort "; } else { diff --git a/CRM/Contact/Form/Search/Custom/Group.php b/CRM/Contact/Form/Search/Custom/Group.php index bfc95e9eba..8c3d7dbe35 100644 --- a/CRM/Contact/Form/Search/Custom/Group.php +++ b/CRM/Contact/Form/Search/Custom/Group.php @@ -228,6 +228,7 @@ class CRM_Contact_Form_Search_Custom_Group extends CRM_Contact_Form_Search_Custo if (!$justIDs) { if (!empty($sort)) { if (is_string($sort)) { + $sort = CRM_Utils_Type::escape($sort, 'String'); $sql .= " ORDER BY $sort "; } else { diff --git a/CRM/Contact/Form/Search/Custom/TagContributions.php b/CRM/Contact/Form/Search/Custom/TagContributions.php index eeb89e9a8a..5ba6a07e03 100644 --- a/CRM/Contact/Form/Search/Custom/TagContributions.php +++ b/CRM/Contact/Form/Search/Custom/TagContributions.php @@ -120,6 +120,7 @@ WHERE $where // Define ORDER BY for query in $sort, with default value if (!empty($sort)) { if (is_string($sort)) { + $sort = CRM_Utils_Type::escape($sort, 'String'); $sql .= " ORDER BY $sort "; } else { diff --git a/CRM/Core/BAO/FinancialTrxn.php b/CRM/Core/BAO/FinancialTrxn.php index 90109b33bd..fe5899fe8d 100644 --- a/CRM/Core/BAO/FinancialTrxn.php +++ b/CRM/Core/BAO/FinancialTrxn.php @@ -128,8 +128,13 @@ class CRM_Core_BAO_FinancialTrxn extends CRM_Financial_DAO_FinancialTrxn { if (!$newTrxn) { $condition = " AND ((ceft1.entity_table IS NOT NULL) OR (cft.payment_instrument_id IS NOT NULL AND ceft1.entity_table IS NULL)) "; } + + if ($orderBy) { + $orderBy = CRM_Utils_Type::escape($orderBy, 'String'); + } + $query = "SELECT ceft.id, ceft.financial_trxn_id FROM `civicrm_financial_trxn` cft -LEFT JOIN civicrm_entity_financial_trxn ceft +LEFT JOIN civicrm_entity_financial_trxn ceft ON ceft.financial_trxn_id = cft.id AND ceft.entity_table = 'civicrm_contribution' LEFT JOIN civicrm_entity_financial_trxn ceft1 ON ceft1.financial_trxn_id = cft.id AND ceft1.entity_table = 'civicrm_financial_item' @@ -137,8 +142,8 @@ LEFT JOIN civicrm_financial_item cfi ON ceft1.entity_table = 'civicrm_financial_ WHERE ceft.entity_id = %1 AND (cfi.entity_table <> 'civicrm_financial_trxn' or cfi.entity_table is NULL) {$condition} ORDER BY cft.id {$orderBy} -LIMIT 1;"; - +LIMIT 1;"; + $params = array(1 => array($entity_id, 'Integer')); $dao = CRM_Core_DAO::executeQuery($query, $params); if ($dao->fetch()) { @@ -163,7 +168,7 @@ LIMIT 1;"; static function getFinancialTrxnTotal($entity_id) { $query = " SELECT (ft.amount+SUM(ceft.amount)) AS total FROM civicrm_entity_financial_trxn AS ft -LEFT JOIN civicrm_entity_financial_trxn AS ceft ON ft.financial_trxn_id = ceft.entity_id +LEFT JOIN civicrm_entity_financial_trxn AS ceft ON ft.financial_trxn_id = ceft.entity_id WHERE ft.entity_table = 'civicrm_contribution' AND ft.entity_id = %1 "; @@ -236,7 +241,7 @@ WHERE ef2.financial_trxn_id =%1 static function getFinancialTrxnLineTotal($entity_id, $entity_table = 'civicrm_contribution') { $query = "SELECT lt.price_field_value_id AS id, ft.financial_trxn_id,ft.amount AS amount FROM civicrm_entity_financial_trxn AS ft LEFT JOIN civicrm_financial_item AS fi ON fi.id = ft.entity_id AND fi.entity_table = 'civicrm_line_item' AND ft.entity_table = 'civicrm_financial_item' -LEFT JOIN civicrm_line_item AS lt ON lt.id = fi.entity_id AND lt.entity_table = %2 +LEFT JOIN civicrm_line_item AS lt ON lt.id = fi.entity_id AND lt.entity_table = %2 WHERE lt.entity_id = %1 "; $sqlParams = array(1 => array($entity_id, 'Integer'), 2 => array($entity_table, 'String')); @@ -261,11 +266,11 @@ WHERE lt.entity_id = %1 "; */ static function deleteFinancialTrxn($entity_id) { $query = "DELETE ceft1, cfi, ceft, cft FROM `civicrm_financial_trxn` cft -LEFT JOIN civicrm_entity_financial_trxn ceft +LEFT JOIN civicrm_entity_financial_trxn ceft ON ceft.financial_trxn_id = cft.id AND ceft.entity_table = 'civicrm_contribution' LEFT JOIN civicrm_entity_financial_trxn ceft1 ON ceft1.financial_trxn_id = cft.id AND ceft1.entity_table = 'civicrm_financial_item' -LEFT JOIN civicrm_financial_item cfi +LEFT JOIN civicrm_financial_item cfi ON ceft1.entity_table = 'civicrm_financial_item' and cfi.id = ceft1.entity_id WHERE ceft.entity_id = %1"; CRM_Core_DAO::executeQuery($query, array(1 => array($entity_id, 'Integer'))); @@ -282,7 +287,7 @@ WHERE ceft.entity_id = %1"; if ((!CRM_Utils_Array::value('financial_type_id', $params) || !CRM_Utils_Array::value('contributionId', $params)) && !CRM_Utils_Array::value('oldPremium', $params)) { return; } - + if (CRM_Utils_Array::value('cost', $params)) { $contributionStatuses = CRM_Contribute_PseudoConstant::contributionStatus(NULL, 'name'); $financialAccountType = CRM_Contribute_PseudoConstant::financialAccountType($params['financial_type_id']); @@ -343,7 +348,7 @@ WHERE ceft.entity_id = %1"; $params['trxnParams']['from_financial_account_id'] = $params['to_financial_account_id']; $params['trxnParams']['to_financial_account_id'] = $financialAccount; $params['trxnParams']['total_amount'] = $amount; - $params['trxnParams']['fee_amount'] = + $params['trxnParams']['fee_amount'] = $params['trxnParams']['net_amount'] = 0; $params['trxnParams']['status_id'] = CRM_Core_OptionGroup::getValue('contribution_status','Completed','name'); $params['trxnParams']['contribution_id'] = isset($params['contribution']->id) ? $params['contribution']->id : $params['contribution_id']; @@ -352,7 +357,7 @@ WHERE ceft.entity_id = %1"; $financialTrxnID = CRM_Core_BAO_FinancialTrxn::getFinancialTrxnId($params['trxnParams']['contribution_id'], 'DESC'); $params['entity_id'] = $financialTrxnID['financialTrxnId']; } - $fItemParams = + $fItemParams = array( 'financial_account_id' => $financialAccount, 'contact_id' => CRM_Core_DAO::getFieldValue('CRM_Core_DAO_Domain', $domainId, 'contact_id'), diff --git a/CRM/Mailing/Event/BAO/Bounce.php b/CRM/Mailing/Event/BAO/Bounce.php index 10d4ae842e..ce6647a197 100644 --- a/CRM/Mailing/Event/BAO/Bounce.php +++ b/CRM/Mailing/Event/BAO/Bounce.php @@ -230,6 +230,7 @@ class CRM_Mailing_Event_BAO_Bounce extends CRM_Mailing_Event_DAO_Bounce { $orderBy = "sort_name ASC, {$bounce}.time_stamp DESC"; if ($sort) { if (is_string($sort)) { + $sort = CRM_Utils_Type::escape($sort, 'String'); $orderBy = $sort; } else { diff --git a/CRM/Mailing/Event/BAO/Delivered.php b/CRM/Mailing/Event/BAO/Delivered.php index 6c01049559..35bfd3a881 100644 --- a/CRM/Mailing/Event/BAO/Delivered.php +++ b/CRM/Mailing/Event/BAO/Delivered.php @@ -195,6 +195,7 @@ class CRM_Mailing_Event_BAO_Delivered extends CRM_Mailing_Event_DAO_Delivered { $orderBy = "sort_name ASC, {$delivered}.time_stamp DESC"; if ($sort) { if (is_string($sort)) { + $sort = CRM_Utils_Type::escape($sort, 'String'); $orderBy = $sort; } else { diff --git a/CRM/Mailing/Event/BAO/Forward.php b/CRM/Mailing/Event/BAO/Forward.php index 9f479685f5..f37883cfe1 100644 --- a/CRM/Mailing/Event/BAO/Forward.php +++ b/CRM/Mailing/Event/BAO/Forward.php @@ -331,6 +331,7 @@ class CRM_Mailing_Event_BAO_Forward extends CRM_Mailing_Event_DAO_Forward { $orderBy = "sort_name ASC, {$forward}.time_stamp DESC"; if ($sort) { if (is_string($sort)) { + $sort = CRM_Utils_Type::escape($sort, 'String'); $orderBy = $sort; } else { diff --git a/CRM/Mailing/Event/BAO/Opened.php b/CRM/Mailing/Event/BAO/Opened.php index 0f434ed5b6..3939859043 100644 --- a/CRM/Mailing/Event/BAO/Opened.php +++ b/CRM/Mailing/Event/BAO/Opened.php @@ -169,11 +169,11 @@ class CRM_Mailing_Event_BAO_Opened extends CRM_Mailing_Event_DAO_Opened { if (!empty($job_id)) { $query .= " AND $job.id = " . CRM_Utils_Type::escape($job_id, 'Integer'); } - + if (!empty($contact_id)) { $query .= " AND $contact.id = " . CRM_Utils_Type::escape($contact_id, 'Integer'); } - + if ($is_distinct) { $query .= " GROUP BY $queue.id "; } @@ -181,6 +181,7 @@ class CRM_Mailing_Event_BAO_Opened extends CRM_Mailing_Event_DAO_Opened { $orderBy = "sort_name ASC, {$open}.time_stamp DESC"; if ($sort) { if (is_string($sort)) { + $sort = CRM_Utils_Type::escape($sort, 'String'); $orderBy = $sort; } else { diff --git a/CRM/Mailing/Event/BAO/Queue.php b/CRM/Mailing/Event/BAO/Queue.php index 14f220ce72..9a9bec5fb4 100644 --- a/CRM/Mailing/Event/BAO/Queue.php +++ b/CRM/Mailing/Event/BAO/Queue.php @@ -114,10 +114,10 @@ class CRM_Mailing_Event_BAO_Queue extends CRM_Mailing_Event_DAO_Queue { public static function getEmailAddress($queue_id) { $email = CRM_Core_BAO_Email::getTableName(); $eq = self::getTableName(); - $query = " SELECT $email.email as email - FROM $email - INNER JOIN $eq - ON $eq.email_id = $email.id + $query = " SELECT $email.email as email + FROM $email + INNER JOIN $eq + ON $eq.email_id = $email.id WHERE $eq.id = " . CRM_Utils_Type::rule($queue_id, 'Integer'); $q = new CRM_Mailing_Event_BAO_Queue(); @@ -190,6 +190,7 @@ class CRM_Mailing_Event_BAO_Queue extends CRM_Mailing_Event_DAO_Queue { $orderBy = "sort_name ASC, {$job}.start_date DESC"; if ($sort) { if (is_string($sort)) { + $sort = CRM_Utils_Type::escape($sort, 'String'); $orderBy = $sort; } else { diff --git a/CRM/Mailing/Event/BAO/Reply.php b/CRM/Mailing/Event/BAO/Reply.php index 59034e1baa..aba6052ddf 100644 --- a/CRM/Mailing/Event/BAO/Reply.php +++ b/CRM/Mailing/Event/BAO/Reply.php @@ -396,6 +396,7 @@ class CRM_Mailing_Event_BAO_Reply extends CRM_Mailing_Event_DAO_Reply { $orderBy = "sort_name ASC, {$reply}.time_stamp DESC"; if ($sort) { if (is_string($sort)) { + $sort = CRM_Utils_Type::escape($sort, 'String'); $orderBy = $sort; } else { diff --git a/CRM/Mailing/Event/BAO/TrackableURLOpen.php b/CRM/Mailing/Event/BAO/TrackableURLOpen.php index ba7e7b247d..fa17c73dda 100644 --- a/CRM/Mailing/Event/BAO/TrackableURLOpen.php +++ b/CRM/Mailing/Event/BAO/TrackableURLOpen.php @@ -223,6 +223,7 @@ class CRM_Mailing_Event_BAO_TrackableURLOpen extends CRM_Mailing_Event_DAO_Track $orderBy = "sort_name ASC, {$click}.time_stamp DESC"; if ($sort) { if (is_string($sort)) { + $sort = CRM_Utils_Type::escape($sort, 'String'); $orderBy = $sort; } else { diff --git a/CRM/Mailing/Event/BAO/Unsubscribe.php b/CRM/Mailing/Event/BAO/Unsubscribe.php index 1910ab37a2..4555c12c14 100644 --- a/CRM/Mailing/Event/BAO/Unsubscribe.php +++ b/CRM/Mailing/Event/BAO/Unsubscribe.php @@ -528,6 +528,7 @@ WHERE email = %2 $orderBy = "sort_name ASC, {$unsub}.time_stamp DESC"; if ($sort) { if (is_string($sort)) { + $sort = CRM_Utils_Type::escape($sort, 'String'); $orderBy = $sort; } else { -- 2.25.1