From 6188a793560673dd0fbd72a683d16d51e0b83a44 Mon Sep 17 00:00:00 2001
From: Patrick Figel <pfigel@greenpeace.org>
Date: Sun, 6 Jan 2019 18:30:30 +0100
Subject: [PATCH] security/core#16 - Smarty - Fix XSS in crmMoney plugin

This fixes an XSS in the crmMoney smarty plugin by checking the
currency against the currency list and adds some basic tests.

Fixes security/core#16
---
 CRM/Utils/Money.php                           |  7 ++++
 .../CRM/Core/Smarty/plugins/CrmMoneyTest.php  | 42 +++++++++++++++++++
 tests/phpunit/CRM/Utils/MoneyTest.php         | 15 +++++++
 3 files changed, 64 insertions(+)
 create mode 100644 tests/phpunit/CRM/Core/Smarty/plugins/CrmMoneyTest.php

diff --git a/CRM/Utils/Money.php b/CRM/Utils/Money.php
index f8afa6cf5c..6f95398353 100644
--- a/CRM/Utils/Money.php
+++ b/CRM/Utils/Money.php
@@ -95,6 +95,13 @@ class CRM_Utils_Money {
     if (!$currency) {
       $currency = $config->defaultCurrency;
     }
+
+    // ensure $currency is a valid currency code
+    // for backwards-compatibility, also accept one space instead of a currency
+    if ($currency != ' ' && !array_key_exists($currency, self::$_currencySymbols)) {
+      throw new CRM_Core_Exception("Invalid currency \"{$currency}\"");
+    }
+
     $amount = self::formatNumericByFormat($amount, $valueFormat);
     // If it contains tags, means that HTML was passed and the
     // amount is already converted properly,
diff --git a/tests/phpunit/CRM/Core/Smarty/plugins/CrmMoneyTest.php b/tests/phpunit/CRM/Core/Smarty/plugins/CrmMoneyTest.php
new file mode 100644
index 0000000000..1a2f62f7fa
--- /dev/null
+++ b/tests/phpunit/CRM/Core/Smarty/plugins/CrmMoneyTest.php
@@ -0,0 +1,42 @@
+<?php
+
+/**
+ * Class CRM_Core_Smarty_plugins_CrmMoneyTest
+ * @group headless
+ */
+class CRM_Core_Smarty_plugins_CrmMoneyTest extends CiviUnitTestCase {
+  public function setUp() {
+    parent::setUp();
+    require_once 'CRM/Core/Smarty.php';
+
+    // Templates should normally be file names, but for unit-testing it's handy to use "string:" notation
+    require_once 'CRM/Core/Smarty/resources/String.php';
+    civicrm_smarty_register_string_resource();
+  }
+
+  /**
+   * @return array
+   */
+  public function moneyCases() {
+    $cases = [];
+    $cases[] = ['$ 4.00', '{assign var="amount" value="4.00"}{$amount|crmMoney:USD}'];
+    $cases[] = ['€ 1,234.00', '{assign var="amount" value="1234.00"}{$amount|crmMoney:EUR}'];
+    $cases[] = [
+      '$ <input size="10" style="background-color:#EBECE4" readonly="readonly" name="eachPaymentAmount" type="text" id="eachPaymentAmount" class="crm-form-text">',
+      '{assign var="amount" value=\'<input size="10" style="background-color:#EBECE4" readonly="readonly" name="eachPaymentAmount" type="text" id="eachPaymentAmount" class="crm-form-text">\'}{$amount|crmMoney:USD}'
+    ];
+    return $cases;
+  }
+
+  /**
+   * @dataProvider moneyCases
+   * @param $expected
+   * @param $input
+   */
+  public function testMoney($expected, $input) {
+    $smarty = CRM_Core_Smarty::singleton();
+    $actual = $smarty->fetch('string:' . $input);
+    $this->assertEquals($expected, $actual, "Process input=[$input]");
+  }
+
+}
diff --git a/tests/phpunit/CRM/Utils/MoneyTest.php b/tests/phpunit/CRM/Utils/MoneyTest.php
index d8b8b6635a..bd343e5171 100644
--- a/tests/phpunit/CRM/Utils/MoneyTest.php
+++ b/tests/phpunit/CRM/Utils/MoneyTest.php
@@ -71,4 +71,19 @@ class CRM_Utils_MoneyTest extends CiviUnitTestCase {
     $this->setCurrencySeparators(',');
   }
 
+  /**
+   * Test that using the space character as a currency works
+   */
+  public function testSpaceCurrency() {
+    $this->assertEquals('  8,950.37', CRM_Utils_Money::format(8950.37, ' '));
+  }
+
+  /**
+   * Test that passing an invalid currency throws an error
+   */
+  public function testInvalidCurrency() {
+    $this->setExpectedException(CRM_Core_Exception::class, 'Invalid currency "NOT_A_CURRENCY"');
+    CRM_Utils_Money::format(4.00, 'NOT_A_CURRENCY');
+  }
+
 }
-- 
2.25.1