[PPP-5370][Update XSS Vulnerabilities] #5758

mmujahiduddin · 2024-10-30T11:18:25Z

Update XSS Vulnerabilities

buildguy · 2024-10-30T11:35:25Z

Note:

Frogbot also supports Contextual Analysis, Secret Detection, IaC and SAST Vulnerabilities Scanning. This features are included as part of the JFrog Advanced Security package, which isn't enabled on your system.

🐸 JFrog Frogbot

assemblies/pentaho-war/src/main/webapp/js/ajaxslt/dom.js

dcleao · 2024-11-04T11:30:13Z

assemblies/pentaho-war/src/main/webapp/js/google-demo.js

@@ -10,6 +10,9 @@
 * Change Date: 2028-08-13
 ******************************************************************************/

+define(['common-ui/util/xss'],


AFAICT, this file is not referenced anywhere.

It appears to demonstrate use of some Google Maps component (GeoCoder) via Pentaho XActions. XActions are essentially deprecated.

Unless you can find a reference to it, solving the XSS issue by removing the file.

Not addressed yet.

dcleao · 2024-11-04T11:31:54Z

assemblies/pentaho-war/src/main/webapp/js/options.js

@@ -20,11 +23,11 @@ function runInBackground( url, target )
 		url = url + "&background=true";
 		if ( target.toLowerCase().indexOf( 'new' ) >= 0 )
 		{
-			var targetWin = window.open( url );
+			var targetWin = window.open(xssUtil.sanitizeHtml(url), 'noopener,noreferrer');


This file is not prepared to be a RequireJS module, and, as such, you should use the global version of the XSS utility library, available under pho.util.xss (and undo the proposed RequireJS modifications).

However, the library does not currently offer any mechanism to validate URLs. AFAIR (but please confirm!), URL protection against XSS consists of ensuring that the URL does not have a javascript protocol. A function such as sanitizeUrl(url) should be added to the XSS utility and then used here.

I am not sure what's the best source to base our implementation of, in this regard. Some pointers:

https://stackoverflow.com/a/15825812/178749

https://cheatsheetseries.owasp.org/cheatsheets/XSS_Filter_Evasion_Cheat_Sheet.html#url-string-evasion

https://github.com/OWASP/owasp-esapi-php/blob/581b993d1d83f7cb925e463a5d1c39fa59425339/src/reference/DefaultSanitizer.php#L115

Regarding this, I think we might be able to use DOMPurify to this effect.

In the DOMPlurify playground page, https://cure53.de/purify, try out the following sample Dirty HTML:

<a href='  javas cript:alert(1)'>I am a dolphin!</a> <a href='http://www.google.com'>I am a dolphin!</a>

It cleans it as:

<a>I am a dolphin!</a> <a href="http://www.google.com">I am a dolphin!</a>

As such, we should be able to implement sanitizeUrl(url) by creating a dummy a element string with the URL as the href attribute: '<a href="' + url + '">'. Then reading back the resulting href value, if any.

Or, we can just base our implementation on their own, from:

https://github.com/cure53/DOMPurify/blob/211ffb5bb94c24b6482b91021b6334295b1152e4/src/purify.ts#L1224

https://github.com/cure53/DOMPurify/blob/211ffb5bb94c24b6482b91021b6334295b1152e4/src/purify.ts#L1303

Which seems like somewhat concise regexp code that we can mimic.

Another aspect which must be considered is whether the suggested sanitizeUrl() method alone would automatically resolve the AppScan finding. I believe the finding is being triggered due to any use of window.open() with a URL string. And so we should need to also create a method such as xssUtil.openWindow(window, url, options) that achieves this in a safe way (by internally sanitizing the URL and hiding the call to window.open).

While the new code is already using the new sanitizeUrl method, the above request, to use openWindow instead, was still not addressed.

Also notice the window.location line, which would need a new XSS Util method to be automatically resolved by App Scan.

It seems that we need another method to achieve that. Something like:

setWindowLocation: function(url) { window.location = xssUtil.sanitizeUrl(url); }

assemblies/pentaho-war/src/main/webapp/js/parameters.js

assemblies/pentaho-war/src/main/webapp/js/src/html/util.js

dcleao · 2024-11-04T11:41:04Z

assemblies/pentaho-war/src/main/webapp/js/utils.js

- 
-  document.getElementById(datePickerDivID).innerHTML = html;
+
+  xssUtil.setHtml(document.getElementById(datePickerDivID), html);


This file is not prepared to be a RequireJS module, and, as such, you should use the global version of the XSS utility library, available under pho.util.xss (and undo the proposed RequireJS modifications).

As you can see in the code above, there are multiple instances of event handler attributes (e.g. onMouseOut, onClick), which using setHtml would eliminate altogether, breaking the functionality.
Instead, must sanitize/encode the parts used to build the HTML string which may pose a XSS risk.

The arguments year, month and day are already properly handled in a safe manner.

The argument dateFieldName however is being used directly as a JavaScript string (with quotes, double or single, provided outside). It should be escaped before use. We need a a function such as encodeForJavaScript(text) (following, for example, OWASP's functionality of OWASP Encoder) and add it to the XSS utility and then use here.

Then, in this same line, mark the setting of unsafe HTML using xssUtil.setHtmlUnsafe(document.getElementById(datePickerDivID), html); instead.

Note that method getButtonCode also needs to be changed.

Not addressed yet.

dcleao · 2024-11-04T12:34:43Z

user-console/src/main/resources/org/pentaho/mantle/public/home/content/welcome/js/main.js

-		window.open( url );
+		/* noopener and noreferrer: These attributes mitigate the risk of tabnabbing and
+               prevent the new page from accessing the original window’s properties. */
+		window.open(xssUtil.sanitizeHtml(url),'noopener,noreferrer');


This file is not prepared to be a RequireJS module, and, as such, you should use the global version of the XSS utility library, available under pho.util.xss (and undo the proposed RequireJS modifications).

Please, use the new method suggested above, pho.util.xss.sanitizeUrl(..).

Actually, as said elsewhere, please use the proposed new method, pho.util.xss.openWindow in these cases.

dcleao · 2024-11-04T12:35:47Z

user-console/src/main/resources/org/pentaho/mantle/public/home/js/gettingStarted.js

-      window.open(href, "_blank");
+      /* noopener and noreferrer: These attributes mitigate the risk of tabnabbing and
+             prevent the new page from accessing the original window’s properties. */
+      window.open(xssUtil.sanitizeHtml(href), "_blank", 'noopener,noreferrer');


Please, use the new method suggested above, xssUtil.sanitizeUrl(..).

Actually, as said elsewhere, please use the proposed new method, pho.util.xss.openWindow in these cases.

hitachivantarasonarqube · 2024-11-07T09:41:58Z

Quality Gate failed

Failed conditions
36.3% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube

buildguy · 2024-11-07T09:42:31Z

❌ Build failed in 1h 5s

Build command:

mvn clean verify -B -e -Daudit -Djs.no.sandbox

👌 All tests passed!

Tests run: 2724, Failures: 0, Skipped: 5 Test Results

ℹ️ This is an automatic message

dcleao

Apart from some local requests, this PR requires the following new XSS Utility features to be merged:

openWindow
setWindowLocation
encodeForJavaScript

[PPP-5370][Update XSS Vulnerabilities]

c14f53f

mmujahiduddin requested a review from a team as a code owner October 30, 2024 11:18

Update End Of Line

0e882a0

This comment has been minimized.

Sign in to view

dcleao requested changes Nov 4, 2024

View reviewed changes

dcleao mentioned this pull request Nov 6, 2024

[PPP-5353]-XSS Findings For Pentaho-platform-plugin-reporting pentaho/pentaho-platform-plugin-reporting#864

Closed

mmujahiduddin changed the base branch from master to BACKLOG-41215 November 7, 2024 08:42

mmujahiduddin changed the base branch from BACKLOG-41215 to XSS-PPP-5370 November 11, 2024 08:42

mmujahiduddin and others added 2 commits November 11, 2024 14:20

Merge branch 'pentaho:PPP-5370' into PPP-5370

b3b5988

Update Review Comments#1

9c795c4

dcleao requested changes Nov 21, 2024

View reviewed changes

Update Review Comments#2

8bca599

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[PPP-5370][Update XSS Vulnerabilities] #5758

[PPP-5370][Update XSS Vulnerabilities] #5758

mmujahiduddin commented Oct 30, 2024

This comment has been minimized.

buildguy commented Oct 30, 2024

This comment has been minimized.

dcleao Nov 4, 2024

dcleao Nov 21, 2024 •

edited

Loading

dcleao Nov 4, 2024

dcleao Nov 5, 2024 •

edited

Loading

dcleao Nov 5, 2024

dcleao Nov 6, 2024

dcleao Nov 21, 2024

dcleao Nov 21, 2024

dcleao Nov 4, 2024

dcleao Nov 21, 2024

dcleao Nov 4, 2024

dcleao Nov 21, 2024

dcleao Nov 4, 2024

dcleao Nov 21, 2024

hitachivantarasonarqube bot commented Nov 7, 2024

buildguy commented Nov 7, 2024

dcleao left a comment


		document.getElementById(datePickerDivID).innerHTML = html;

		xssUtil.setHtml(document.getElementById(datePickerDivID), html);

[PPP-5370][Update XSS Vulnerabilities] #5758

Are you sure you want to change the base?

[PPP-5370][Update XSS Vulnerabilities] #5758

Conversation

mmujahiduddin commented Oct 30, 2024

This comment has been minimized.

buildguy commented Oct 30, 2024

This comment has been minimized.

Choose a reason for hiding this comment

dcleao Nov 21, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dcleao Nov 5, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hitachivantarasonarqube bot commented Nov 7, 2024

Quality Gate failed

buildguy commented Nov 7, 2024

❌ Build failed in 1h 5s

dcleao left a comment

Choose a reason for hiding this comment

dcleao Nov 21, 2024 •

edited

Loading

dcleao Nov 5, 2024 •

edited

Loading