Fix stored XSS security issue: remove inline JS from URL

francoisjacquet · Jun 3, 2022 · adc5dfe · adc5dfe
1 parent 4a3f7b9
commit adc5dfe
Show file tree

Hide file tree

Showing 2 changed files with 11 additions and 9 deletions.
diff --git a/CHANGES.md b/CHANGES.md
@@ -69,7 +69,7 @@ Changes in 9.0
 - Better format for "Add another marking period" form in EditReportCardGrades.php
 - Fix Improper Access Control security issue: add random string to photo file name in TipMessage.fnc.php, Transcripts.fnc.php, PrintClassPictures.php, Student.php, User.php & General_Info.inc.php, thanks to @dungtuanha
 - Fix stored XSS security issue: decode HTML entities from URL in PreparePHP_SELF.fnc.php, thanks to @khanhchauminh
-- Fix stored XSS security issue: remove inline JS from URL in PreparePHP_SELF.fnc.php, thanks to @intrapus
+- Fix stored XSS security issue: remove inline JS from URL in PreparePHP_SELF.fnc.php, thanks to @intrapus & @domiee13
 - Fix stored XSS security issue: add semicolon to HTML entity so it can be decoded in PreparePHP_SELF.fnc.php, thanks to @intrapus
 - Accessibility: add hidden input label using .a11y-hidden class in ReportCardComments.php, StudentFields.php & Grades/TeacherCompletion.php
 - Accessibility: add select label in Eligibility/TeacherCompletion.php, Student.php, StudentList.php, MassDrops.php & MassSchedule.php

diff --git a/functions/PreparePHP_SELF.fnc.php b/functions/PreparePHP_SELF.fnc.php
@@ -173,7 +173,7 @@ function RedirectURL( $remove )
  */
 function URLEscape( $string )
 {
-	$fixed_entities_string = preg_replace_callback(
+	$string = preg_replace_callback(
 		// Match both decimal & hex code (although hex codes can contain a-f letters).
 		// Should be enough as the alphabet hex codes only have numbers.
 		"/(&#x?[0-9]+;?)/i",
@@ -191,18 +191,20 @@ function( $match ) {
 	);
 
 	// Fix stored XSS security issue: decode HTML entities from URL.
-	$decoded_string = html_entity_decode( (string) $fixed_entities_string );
+	$string = html_entity_decode( (string) $string );
 
 	$remove = [
 		// Fix stored XSS security issue: remove inline JS from URL.
 		'javascript:',
 	];
 
-	$decoded_sanitized_string = str_ireplace(
-		$remove,
-		'',
-		$decoded_string
-	);
+	foreach ( $remove as $remove_string )
+	{
+		while ( strpos( $string, $remove_string ) !== false )
+		{
+			$string = str_ireplace( $remove, '', $string );
+		}
+	}
 
 	$entities = [
 		'%21',
@@ -251,7 +253,7 @@ function( $match ) {
 	return str_replace(
 		$entities,
 		$replacements,
-		rawurlencode( $decoded_sanitized_string )
+		rawurlencode( $string )
 	);
 }