From a93964781fd2630ca34915e2b8dfe9d2122fbf6c Mon Sep 17 00:00:00 2001 From: Tim Otten Date: Thu, 1 Aug 2013 14:19:00 -0700 Subject: [PATCH] CRM-13120 - APIv3 - Don't return "null" from "create" action * Split CRM_Core_HTMLInputCoder in three: * CRM_Utils_API_AbstractFieldCoder * CRM_Utils_API_HTMLInputCoder * CRM_Core_HTMLInputCoder (for backward compatibility) * Add CRM_Utils_API_NullOutputCoder (based on AbstractFieldCoder) ---------------------------------------- * CRM-13120: http://issues.civicrm.org/jira/browse/CRM-13120 --- CRM/Core/HTMLInputCoder.php | 176 +-------------------------- CRM/Utils/API/AbstractFieldCoder.php | 135 ++++++++++++++++++++ CRM/Utils/API/HTMLInputCoder.php | 142 +++++++++++++++++++++ CRM/Utils/API/NullOutputCoder.php | 87 +++++++++++++ api/api.php | 9 +- 5 files changed, 375 insertions(+), 174 deletions(-) create mode 100644 CRM/Utils/API/AbstractFieldCoder.php create mode 100644 CRM/Utils/API/HTMLInputCoder.php create mode 100644 CRM/Utils/API/NullOutputCoder.php diff --git a/CRM/Core/HTMLInputCoder.php b/CRM/Core/HTMLInputCoder.php index 5bfd11fee1..ba7eeb85fe 100644 --- a/CRM/Core/HTMLInputCoder.php +++ b/CRM/Core/HTMLInputCoder.php @@ -24,118 +24,14 @@ | see the CiviCRM license FAQ at http://civicrm.org/licensing | +--------------------------------------------------------------------+ */ - -/** - * This class captures the encoding practices of CRM-5667 in a reusable - * fashion. In this design, all submitted values are partially HTML-encoded - * before saving to the database. If a DB reader needs to output in - * non-HTML medium, then it should undo the partial HTML encoding. - * - * This class should be short-lived -- 4.3 should introduce an alternative - * escaping scheme and consequently remove HTMLInputCoder. - * - * @package CRM - * @copyright CiviCRM LLC (c) 2004-2013 - * $Id$ - * - */ - -require_once 'api/Wrapper.php'; -class CRM_Core_HTMLInputCoder implements API_Wrapper { - private static $skipFields = NULL; - - /** - * @var CRM_Core_HTMLInputCoder - */ - private static $_singleton = NULL; - - /** - * @return CRM_Core_HTMLInputCoder - */ - public static function singleton() { - if (self::$_singleton === NULL) { - self::$_singleton = new CRM_Core_HTMLInputCoder(); - } - return self::$_singleton; - } - - /** - * @return array list of field names - */ - public static function getSkipFields() { - if (self::$skipFields === NULL) { - self::$skipFields = array( - 'widget_code', - 'html_message', - 'body_html', - 'msg_html', - 'description', - 'intro', - 'thankyou_text', - 'tf_thankyou_text', - 'intro_text', - 'page_text', - 'body_text', - 'footer_text', - 'thankyou_footer', - 'thankyou_footer_text', - 'new_text', - 'renewal_text', - 'help_pre', - 'help_post', - 'confirm_title', - 'confirm_text', - 'confirm_footer_text', - 'confirm_email_text', - 'event_full_text', - 'waitlist_text', - 'approval_req_text', - 'report_header', - 'report_footer', - 'cc_id', - 'bcc_id', - 'premiums_intro_text', - 'honor_block_text', - 'pay_later_text', - 'pay_later_receipt', - 'label', // This is needed for FROM Email Address configuration. dgg - 'url', // This is needed for navigation items urls - 'details', - 'msg_text', // message templates’ text versions - 'text_message', // (send an) email to contact’s and CiviMail’s text version - 'data', // data i/p of persistent table - 'sqlQuery', // CRM-6673 - 'pcp_title', - 'pcp_intro_text', - 'new', // The 'new' text in word replacements - ); - } - return self::$skipFields; - } +class CRM_Core_HTMLInputCoder { /** * @param string $fldName * @return bool TRUE if encoding should be skipped for this field */ public static function isSkippedField($fldName) { - $skipFields = self::getSkipFields(); - - // Field should be skipped - if (in_array($fldName, $skipFields)) { - return TRUE; - } - // Field is multilingual and after cutting off _xx_YY should be skipped (CRM-7230)… - if ((preg_match('/_[a-z][a-z]_[A-Z][A-Z]$/', $fldName) && in_array(substr($fldName, 0, -6), $skipFields))) { - return TRUE; - } - // Field can take multiple entries, eg. fieldName[1], fieldName[2], etc. - // We remove the index and check again if the fieldName in the list of skipped fields. - $matches = array(); - if (preg_match('/^(.*)\[\d+\]/', $fldName, $matches) && in_array($matches[1], $skipFields)) { - return TRUE; - } - - return FALSE; + return CRM_Utils_API_HTMLInputCoder::singleton()->isSkippedField($fldName); } /** @@ -147,71 +43,7 @@ class CRM_Core_HTMLInputCoder implements API_Wrapper { * If FALSE, then non-string values will be preserved */ public static function encodeInput(&$values, $castToString = TRUE) { - if (is_array($values)) { - foreach ($values as &$value) { - self::encodeInput($value); - } - } elseif ($castToString || is_string($values)) { - $values = str_replace(array('<', '>'), array('<', '>'), $values); - } - } - - public static function decodeOutput(&$values, $castToString = TRUE) { - if (is_array($values)) { - foreach ($values as &$value) { - self::decodeOutput($value); - } - } elseif ($castToString || is_string($values)) { - $values = str_replace(array('<', '>'), array('<', '>'), $values); - } - } - - /** - * {@inheritDoc} - */ - public function fromApiInput($apiRequest) { - $lowerAction = strtolower($apiRequest['action']); - if ($apiRequest['version'] == 3 && in_array($lowerAction, array('get', 'create'))) { - // note: 'getsingle', 'replace', 'update', and chaining all build on top of 'get'/'create' - foreach ($apiRequest['params'] as $key => $value) { - // Don't apply escaping to API control parameters (e.g. 'api.foo' or 'options.foo') - // and don't apply to other skippable fields - if (!self::isApiControlField($key) && !self::isSkippedField($key)) { - self::encodeInput($apiRequest['params'][$key], FALSE); - } - } - } elseif ($apiRequest['version'] == 3 && $lowerAction == 'setvalue') { - if (isset($apiRequest['params']['field']) && isset($apiRequest['params']['value'])) { - if (!self::isSkippedField($apiRequest['params']['field'])) { - self::encodeInput($apiRequest['params']['value'], FALSE); - } - } - } - return $apiRequest; + return CRM_Utils_API_HTMLInputCoder::singleton()->encodeInput($values, $castToString); } - /** - * {@inheritDoc} - */ - public function toApiOutput($apiRequest, $result) { - $lowerAction = strtolower($apiRequest['action']); - if ($apiRequest['version'] == 3 && in_array($lowerAction, array('get', 'create', 'setvalue'))) { - foreach ($result as $key => $value) { - // Don't apply escaping to API control parameters (e.g. 'api.foo' or 'options.foo') - // and don't apply to other skippable fields - if (!self::isApiControlField($key) && !self::isSkippedField($key)) { - self::decodeOutput($result[$key], FALSE); - } - } - } - // setvalue? - return $result; - } - - /** - * @return bool - */ - protected function isApiControlField($key) { - return (FALSE !== strpos($key, '.')); - } -} +} \ No newline at end of file diff --git a/CRM/Utils/API/AbstractFieldCoder.php b/CRM/Utils/API/AbstractFieldCoder.php new file mode 100644 index 0000000000..77129407aa --- /dev/null +++ b/CRM/Utils/API/AbstractFieldCoder.php @@ -0,0 +1,135 @@ + list of field names + */ + public function getSkipFields() { + return NULL; + } + + /** + * @param string $fldName + * @return bool TRUE if encoding should be skipped for this field + */ + public function isSkippedField($fldName) { + $skipFields = $this->getSkipFields(); + if ($skipFields === NULL) { + return FALSE; + } + + // Field should be skipped + if (in_array($fldName, $skipFields)) { + return TRUE; + } + // Field is multilingual and after cutting off _xx_YY should be skipped (CRM-7230)… + if ((preg_match('/_[a-z][a-z]_[A-Z][A-Z]$/', $fldName) && in_array(substr($fldName, 0, -6), $skipFields))) { + return TRUE; + } + // Field can take multiple entries, eg. fieldName[1], fieldName[2], etc. + // We remove the index and check again if the fieldName in the list of skipped fields. + $matches = array(); + if (preg_match('/^(.*)\[\d+\]/', $fldName, $matches) && in_array($matches[1], $skipFields)) { + return TRUE; + } + + return FALSE; + } + + /** + * This function is going to filter the + * submitted values. + * + * @param array|string $values the field value from the API + */ + public abstract function encodeInput(&$values); + + public abstract function decodeOutput(&$values); + + /** + * {@inheritDoc} + */ + public function fromApiInput($apiRequest) { + $lowerAction = strtolower($apiRequest['action']); + if ($apiRequest['version'] == 3 && in_array($lowerAction, array('get', 'create'))) { + // note: 'getsingle', 'replace', 'update', and chaining all build on top of 'get'/'create' + foreach ($apiRequest['params'] as $key => $value) { + // Don't apply escaping to API control parameters (e.g. 'api.foo' or 'options.foo') + // and don't apply to other skippable fields + if (!$this->isApiControlField($key) && !$this->isSkippedField($key)) { + $this->encodeInput($apiRequest['params'][$key]); + } + } + } + elseif ($apiRequest['version'] == 3 && $lowerAction == 'setvalue') { + if (isset($apiRequest['params']['field']) && isset($apiRequest['params']['value'])) { + if (!$this->isSkippedField($apiRequest['params']['field'])) { + $this->encodeInput($apiRequest['params']['value']); + } + } + } + return $apiRequest; + } + + /** + * {@inheritDoc} + */ + public function toApiOutput($apiRequest, $result) { + $lowerAction = strtolower($apiRequest['action']); + if ($apiRequest['version'] == 3 && in_array($lowerAction, array('get', 'create', 'setvalue'))) { + foreach ($result as $key => $value) { + // Don't apply escaping to API control parameters (e.g. 'api.foo' or 'options.foo') + // and don't apply to other skippable fields + if (!$this->isApiControlField($key) && !$this->isSkippedField($key)) { + $this->decodeOutput($result[$key]); + } + } + } + // setvalue? + return $result; + } + + /** + * @return bool + */ + protected function isApiControlField($key) { + return (FALSE !== strpos($key, '.')); + } +} diff --git a/CRM/Utils/API/HTMLInputCoder.php b/CRM/Utils/API/HTMLInputCoder.php new file mode 100644 index 0000000000..422aa77fa7 --- /dev/null +++ b/CRM/Utils/API/HTMLInputCoder.php @@ -0,0 +1,142 @@ + list of field names + */ + public function getSkipFields() { + if ($this->skipFields === NULL) { + $this->skipFields = array( + 'widget_code', + 'html_message', + 'body_html', + 'msg_html', + 'description', + 'intro', + 'thankyou_text', + 'tf_thankyou_text', + 'intro_text', + 'page_text', + 'body_text', + 'footer_text', + 'thankyou_footer', + 'thankyou_footer_text', + 'new_text', + 'renewal_text', + 'help_pre', + 'help_post', + 'confirm_title', + 'confirm_text', + 'confirm_footer_text', + 'confirm_email_text', + 'event_full_text', + 'waitlist_text', + 'approval_req_text', + 'report_header', + 'report_footer', + 'cc_id', + 'bcc_id', + 'premiums_intro_text', + 'honor_block_text', + 'pay_later_text', + 'pay_later_receipt', + 'label', // This is needed for FROM Email Address configuration. dgg + 'url', // This is needed for navigation items urls + 'details', + 'msg_text', // message templates’ text versions + 'text_message', // (send an) email to contact’s and CiviMail’s text version + 'data', // data i/p of persistent table + 'sqlQuery', // CRM-6673 + 'pcp_title', + 'pcp_intro_text', + 'new', // The 'new' text in word replacements + ); + } + return $this->skipFields; + } + + /** + * This function is going to filter the + * submitted values across XSS vulnerability. + * + * @param array|string $values + * @param bool $castToString If TRUE, all scalars will be filtered (and therefore cast to strings) + * If FALSE, then non-string values will be preserved + */ + public function encodeInput(&$values, $castToString = FALSE) { + if (is_array($values)) { + foreach ($values as &$value) { + $this->encodeInput($value, TRUE); + } + } elseif ($castToString || is_string($values)) { + $values = str_replace(array('<', '>'), array('<', '>'), $values); + } + } + + public function decodeOutput(&$values, $castToString = FALSE) { + if (is_array($values)) { + foreach ($values as &$value) { + $this->decodeOutput($value, TRUE); + } + } elseif ($castToString || is_string($values)) { + $values = str_replace(array('<', '>'), array('<', '>'), $values); + } + } +} diff --git a/CRM/Utils/API/NullOutputCoder.php b/CRM/Utils/API/NullOutputCoder.php new file mode 100644 index 0000000000..0de35a416f --- /dev/null +++ b/CRM/Utils/API/NullOutputCoder.php @@ -0,0 +1,87 @@ +decodeOutput($value, TRUE); + } + } + elseif ($castToString || is_string($values)) { + if ($values === 'null') { + $values = ''; + } + } + } + + public function toApiOutput($apiRequest, $result) { + $lowerAction = strtolower($apiRequest['action']); + if ($lowerAction === 'create') { + return parent::toApiOutput($apiRequest, $result); + } else { + return $result; + } + } +} diff --git a/api/api.php b/api/api.php index 5062422df6..76637548ce 100644 --- a/api/api.php +++ b/api/api.php @@ -19,7 +19,10 @@ * array to be passed to function */ function civicrm_api($entity, $action, $params, $extra = NULL) { - $apiWrappers = array(CRM_Core_HTMLInputCoder::singleton()); + $apiWrappers = array( + CRM_Utils_API_HTMLInputCoder::singleton(), + CRM_Utils_API_NullOutputCoder::singleton() + ); try { require_once ('api/v3/utils.php'); require_once 'api/Exception.php'; @@ -58,6 +61,7 @@ function civicrm_api($entity, $action, $params, $extra = NULL) { civicrm_api3_verify_mandatory($apiRequest['params'], NULL, _civicrm_api3_getrequired($apiRequest)); } + // For input filtering, process $apiWrappers in forward order foreach ($apiWrappers as $apiWrapper) { $apiRequest = $apiWrapper->fromApiInput($apiRequest); } @@ -78,7 +82,8 @@ function civicrm_api($entity, $action, $params, $extra = NULL) { return civicrm_api3_create_error("API (" . $apiRequest['entity'] . "," . $apiRequest['action'] . ") does not exist (join the API team and implement it!)"); } - foreach ($apiWrappers as $apiWrapper) { + // For output filtering, process $apiWrappers in reverse order + foreach (array_reverse($apiWrappers) as $apiWrapper) { $result = $apiWrapper->toApiOutput($apiRequest, $result); } -- 2.25.1