Institution Sanitation issue

- Remove sanitation off outbound notes content to avoid interfering with embedded html tags that are added when this field is appended with GriSciColl info.
- Remove sanitation notes which were only meant to communicate to internal team when sanitation content was originally added.
Resolves following issue, in part: #1982

classes/InstitutionManager.php

@@ -24,8 +24,22 @@ public function getInstitutionData(){
 				$stmt->bind_param('i', $this->iid);
 				$stmt->execute();
 				$rs = $stmt->get_result();
-				while($r = $rs->fetch_assoc()){
-					$retArr = $r;
+				if($r = $rs->fetch_object()){
+					$retArr['iid'] = $r->iid;
+					$retArr['institutioncode'] = $this->cleanOutStr($r->institutioncode);
+					$retArr['institutionname'] = $this->cleanOutStr($r->institutionname);
+					$retArr['institutionname2'] = $this->cleanOutStr($r->institutionname2);
+					$retArr['address1'] = $this->cleanOutStr($r->address1);
+					$retArr['address2'] = $this->cleanOutStr($r->address2);
+					$retArr['city'] = $this->cleanOutStr($r->city);
+					$retArr['stateprovince'] = $this->cleanOutStr($r->stateprovince);
+					$retArr['postalcode'] = $this->cleanOutStr($r->postalcode);
+					$retArr['country'] = $this->cleanOutStr($r->country);
+					$retArr['phone'] = $this->cleanOutStr($r->phone);
+					$retArr['contact'] = $this->cleanOutStr($r->contact);
+					$retArr['email'] = $this->cleanOutStr($r->email);
+					$retArr['url'] = $this->cleanOutStr($r->url);
+					$retArr['notes'] = $r->notes;	//Do not sanitize at this time. Html tags are included when content is added from GBIF's GrSciColl. Wait until we have resolve the html sanitation issue.
 				}
 				$rs->free();
 				$stmt->close();

collections/misc/institutioneditor.php

@@ -6,36 +6,23 @@
 
 if(!$SYMB_UID) header('Location: ../../profile/index.php?refurl=../collections/admin/institutioneditor.php?' . htmlspecialchars($_SERVER['QUERY_STRING'], ENT_QUOTES));
 
-//since integers are sanizated here, we don't need to resanitize using htmlspecialchars
 $iid = array_key_exists('iid', $_REQUEST) ? filter_var($_REQUEST['iid'], FILTER_SANITIZE_NUMBER_INT) : '';
 $targetCollid = array_key_exists('targetcollid', $_REQUEST) ? filter_var($_REQUEST['targetcollid'], FILTER_SANITIZE_NUMBER_INT) : '';
-
-//emode is alway 0 or 1, thus we can explicit set to 1 whenever emode is true. No sanitation needed because value is not set as input value, which could be anything.
 $eMode = !empty($_REQUEST['emode']) ? 1 : 0;
-
-//$instCodeDefault this does need to be sanitized, but better to do further down where it is used as output. But OK to sanitize here, and maybe better if used numerous times as output.
-//But not if it is set within the class to be used within an SQL statement. Then it's needs to be sanitized beyond the statements that send it to the class
 $instCodeDefault = array_key_exists('instcode',$_REQUEST) ? $_REQUEST['instcode'] : '';
-
-// $formSubmit does not have to be sanitized because it's only used within condition statements and not output to page
 $formSubmit = array_key_exists('formsubmit', $_POST) ? $_POST['formsubmit'] : '';
 
 $instManager = new InstitutionManager();
-
-// text with $fullCollList is sanitized using cleanOutStr (including htmlspecialchars) within getCollectionList function.
-// The iid value does not have to be sanitized because it is defined as an int, but CheckMarx probably won't know this a might have a problem
 $fullCollList = $instManager->getCollectionList();
 $instManager->setInstitutionId($iid);
 
 //Create a list of collection that are linked to this institutions
 $collList = array();
 foreach($fullCollList as $k => $v){
-	//Values were already sanitized when $fullCollList was obtained from DB, thus we should not double sanitized here (e.g. & becomes &
 	if($v['iid'] == $iid) $collList[$k] = $v['name'];
 }
 
 $editorCode = 0;
-$statusStr = '';
 if($IS_ADMIN){
 	$editorCode = 3;
 }
@@ -45,10 +32,10 @@
 		$editorCode = 2;
 	}
 }
+$statusStr = '';
 if($editorCode){
 	if($formSubmit == 'Add Institution'){
 		if($instManager->insertInstitution($_POST)){
-			// Can only be set to int within class. Let's see if checkmarx knows this.
 			$iid = $instManager->getInstitutionId();
 			$statusStr = 'SUCCESS, institution added!';
 			if($targetCollid) header('Location: ../misc/collprofiles.php?collid=' . $targetCollid);
@@ -188,17 +175,13 @@ function validateAddCollectionForm(f){
 		?>
 		<hr />
 		<div style="margin:20px;color:<?php echo (substr($statusStr,0,5)=='ERROR'?'red':'green'); ?>;">
-			<?php //$statusStr is only output from db engine, thus not a big threat, but Checkmarx won't know that, and better safe than sorry ?>
 			<?= htmlspecialchars($statusStr, ENT_COMPAT | ENT_HTML401 | ENT_SUBSTITUTE) ?>
 		</div>
 		<hr />
 		<?php
 	}
 	if($iid){
 		if($instArr = $instManager->getInstitutionData()){
-			//Cleaning this in function when data is harvested is fine, but only if data is used as output by all pages.
-			//If data from fuction is used by another class/page to be reinserted into data, than that would be a problem, but this is rare.
-			$instArr = $instManager->cleanOutArray($instArr);
 			?>
 			<div style="float:right;">
 				<a href="institutioneditor.php">
@@ -388,7 +371,6 @@ function validateAddCollectionForm(f){
 						<div>
 							<?php
 							if($collList){
-								//$collName is cleaned within function that built data array
 								foreach($collList as $id => $collName){
 									echo '<div style="margin:5px;font-weight:bold;clear:both;height:15px;">';
 									echo '<div style="float:left;"><a href="../misc/collprofiles.php?collid=' . $id . '">' . $collName . '</a></div> ';
@@ -407,7 +389,6 @@ function validateAddCollectionForm(f){
 							<?php
 							//Don't show collection that already linked and only show one that user can admin
 							$addList = array();
-							//$fullCollList cleaned when created
 							foreach($fullCollList as $collid => $collArr){
 								if($collArr['iid'] != $iid){
 									if($IS_ADMIN || (isset($USER_RIGHTS["CollAdmin"]) && in_array($collid,$USER_RIGHTS["CollAdmin"]))){