Commit 50ed845a authored by Seamus Lee's avatar Seamus Lee

security/core#49 Ensure that only intergers are passed to the IN build options in address

Fix Rule checking and add a unit test

Add in unit test on building country_id options too

Add in a unit test for building county options with a state_province_id filter
parent fa7ac2f8
......@@ -1319,6 +1319,9 @@ SELECT is_primary,
}
}
if (!empty($props['country_id'])) {
if (!CRM_Utils_Rule::commaSeparatedIntegers(implode(',', (array) $props['country_id']))) {
throw new CRM_Core_Exception(ts('Province limit or default country setting is incorrect'));
}
$params['condition'] = 'country_id IN (' . implode(',', (array) $props['country_id']) . ')';
}
break;
......@@ -1331,6 +1334,9 @@ SELECT is_primary,
if ($context != 'get' && $context != 'validate') {
$config = CRM_Core_Config::singleton();
if (!empty($config->countryLimit) && is_array($config->countryLimit)) {
if (!CRM_Utils_Rule::commaSeparatedIntegers(implode(',', $config->countryLimit))) {
throw new CRM_Core_Exception(ts('Available Country setting is incorrect'));
}
$params['condition'] = 'id IN (' . implode(',', $config->countryLimit) . ')';
}
}
......@@ -1339,6 +1345,9 @@ SELECT is_primary,
// Filter county list based on chosen state
case 'county_id':
if (!empty($props['state_province_id'])) {
if (!CRM_Utils_Rule::commaSeparatedIntegers(implode(',', (array) $props['state_province_id']))) {
throw new CRM_Core_Exception(ts('Can only accept Integers for state_province_id filtering'));
}
$params['condition'] = 'state_province_id IN (' . implode(',', (array) $props['state_province_id']) . ')';
}
break;
......
......@@ -488,6 +488,8 @@ class CRM_Utils_Rule {
*/
public static function commaSeparatedIntegers($value) {
foreach (explode(',', $value) as $val) {
// Remove any Whitespace around the key.
$val = trim($val);
if (!self::positiveInteger($val)) {
return FALSE;
}
......
......@@ -479,4 +479,47 @@ class api_v3_AddressTest extends CiviUnitTestCase {
$this->assertEquals($expectState, $created['state_province_id']);
}
public function testBuildStateProvinceOptionsWithDodgyProvinceLimit() {
$provinceLimit = [1228, "abcd;ef"];
$this->callAPISuccess('setting', 'create', [
'provinceLimit' => $provinceLimit,
]);
$result = $this->callAPIFailure('address', 'getoptions', ['field' => 'state_province_id']);
// confirm that we hit our error not a SQLI.
$this->assertEquals('Province limit or default country setting is incorrect', $result['error_message']);
$this->callAPISuccess('setting', 'create', [
'provinceLimit' => [1228],
]);
// Now confirm with a correct province setting it works fine
$this->callAPISuccess('address', 'getoptions', ['field' => 'state_province_id']);
}
public function testBuildCountryWithDodgyCountryLimitSetting() {
$countryLimit = [1228, "abcd;ef"];
$this->callAPISuccess('setting', 'create', [
'countryLimit' => $countryLimit,
]);
$result = $this->callAPIFailure('address', 'getoptions', ['field' => 'country_id']);
// confirm that we hit our error not a SQLI.
$this->assertEquals('Available Country setting is incorrect', $result['error_message']);
$this->callAPISuccess('setting', 'create', [
'countryLimit' => [1228],
]);
// Now confirm with a correct province setting it works fine
$this->callAPISuccess('address', 'getoptions', ['field' => 'country_id']);
}
public function testBuildCountyWithDodgeStateProvinceFiltering() {
$result = $this->callAPIFailure('Address', 'getoptions', [
'field' => 'county_id',
'state_province_id' => "abcd;ef",
]);
$this->assertEquals('Can only accept Integers for state_province_id filtering', $result['error_message']);
$goodResult = $this->callAPISuccess('Address', 'getoptions', [
'field' => 'county_id',
'state_province_id' => 1004,
]);
$this->assertEquals('San Francisco', $goodResult['values'][4]);
}
}
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment