Skip to content

Commit

Permalink
CRM-21753 proposal for json validation
Browse files Browse the repository at this point in the history
Per #11658

the dedupe code has a mechanism to handle a nuanced array of criteria.

This PR is purely to discuss the passing of that criteria through the UI, with
a proposed validation rule.

The criteria in the url might look like

criteria={"contact" : {"first_name" :{"IN" : ["Peter", "Paul", "Mary"]]]}

This is passed to the contact api to limit the contacts to look for matches
for, prior to applying permissions (which is done when actually finding the
matches.

My specific concern is that the criteria is passed back to the template and
used in constructed urls - so I am looking to validate it on the way in
for xss in this PR.
  • Loading branch information
eileenmcnaughton committed Feb 14, 2018
1 parent 951ab9f commit b4b0b4a
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 2 deletions.
7 changes: 5 additions & 2 deletions CRM/Contact/Page/DedupeFind.php
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,9 @@ public function run() {
$limit = CRM_Utils_Request::retrieve('limit', 'Integer', $this);
$rgid = CRM_Utils_Request::retrieve('rgid', 'Positive', $this);
$cid = CRM_Utils_Request::retrieve('cid', 'Positive', $this, FALSE, 0);
// Using a placeholder for criteria as it is intended to be able to pass this later.
$criteria = array();
$criteria = CRM_Utils_Request::retrieve('criteria', 'Json', $this, FALSE);
$this->assign('criteria', $criteria);

$isConflictMode = ($context == 'conflicts');
if ($cid) {
$this->_cid = $cid;
Expand All @@ -79,8 +80,10 @@ public function run() {
'rgid' => $rgid,
'gid' => $gid,
'limit' => $limit,
'criteria' => $criteria,
);
$this->assign('urlQuery', CRM_Utils_System::makeQueryString($urlQry));
$criteria = json_decode($criteria, TRUE);

if ($context == 'search') {
$context = 'search';
Expand Down
39 changes: 39 additions & 0 deletions CRM/Utils/Rule.php
Original file line number Diff line number Diff line change
Expand Up @@ -813,6 +813,25 @@ public static function xssString($value) {
}
}

/**
* Validate json string for xss
*
* @param string $value
*
* @return bool
* False if invalid, true if valid / safe.
*/
public static function json($value) {
if (!self::xssString($value)) {
return FALSE;
}
$array = json_decode($value, TRUE);
if (!$array || !is_array($array)) {
return FALSE;
}
return self::arrayValue($array);
}

/**
* @param $path
*
Expand Down Expand Up @@ -942,4 +961,24 @@ public static function checkExtesnionKeyIsValid($key = NULL) {
return TRUE;
}

/**
* Validate array recursively checking keys and values.
*
* @param array $array
* @return bool
*/
protected static function arrayValue($array) {
foreach ($array as $key => $item) {
if (is_array($item)) {
if (!self::xssString($key) || !self::arrayValue($item)) {
return FALSE;
}
}
if (!self::xssString($key) || !self::xssString($item)) {
return FALSE;
}
}
return TRUE;
}

}
7 changes: 7 additions & 0 deletions CRM/Utils/Type.php
Original file line number Diff line number Diff line change
Expand Up @@ -422,6 +422,7 @@ public static function validate($data, $type, $abort = TRUE, $name = 'One of par
'MysqlOrderByDirection',
'MysqlOrderBy',
'ExtensionKey',
'Json',
);
if (!in_array($type, $possibleTypes)) {
if ($isThrowException) {
Expand Down Expand Up @@ -530,6 +531,12 @@ public static function validate($data, $type, $abort = TRUE, $name = 'One of par
return $data;
}
break;

case 'Json':
if (CRM_Utils_Rule::json($data)) {
return $data;
}
break;
}

if ($abort) {
Expand Down

0 comments on commit b4b0b4a

Please sign in to comment.