-
Notifications
You must be signed in to change notification settings - Fork 148
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[PPP-5353]-XSS Findings For Pentaho-platform-plugin-reporting #874
[PPP-5353]-XSS Findings For Pentaho-platform-plugin-reporting #874
Conversation
This comment has been minimized.
This comment has been minimized.
e3d57d8
to
997c685
Compare
This comment has been minimized.
This comment has been minimized.
997c685
to
4f72607
Compare
This comment has been minimized.
This comment has been minimized.
4f72607
to
3b8ea76
Compare
This comment has been minimized.
This comment has been minimized.
@@ -818,7 +818,7 @@ define([ 'common-ui/util/util', 'common-ui/util/timeutil', 'common-ui/util/forma | |||
if(isRunningIFrameInSameOrigin) { | |||
if (!top.mantle_initialized) { | |||
this._topMantleOpenTabRegistration = top.mantle_openTab = function(name, title, url) { | |||
window.open(url, '_blank'); | |||
window.open(xssUtil.sanitizeUrl(url), '_blank'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should use the new xssUtil.openWindow(..)
method instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated
@@ -1189,7 +1189,7 @@ define([ 'common-ui/util/util', 'common-ui/util/timeutil', 'common-ui/util/forma | |||
hideDlgAndPane(registry.byId('feedbackScreen')); | |||
|
|||
//Show loading screen | |||
$('#notification-message').html(_Messages.getString('LoadingPage') + " " + mainJobStatus.page + " " + _Messages.getString('Of') + " " + mainJobStatus.totalPages); | |||
$('#notification-message').html(xssUtil.sanitizeHtml(_Messages.getString('LoadingPage') + " " + mainJobStatus.page + " " + _Messages.getString('Of') + " " + mainJobStatus.totalPages)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should use the xssUtil.setHtml(..)
method, after support for jQuery objects has been added to setHtml(..)
and setHtmlUnsafe(..)
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See #864 (comment).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated
@@ -1232,7 +1232,7 @@ define([ 'common-ui/util/util', 'common-ui/util/timeutil', 'common-ui/util/forma | |||
isPageCountUpdated = true; | |||
} | |||
|
|||
$('#notification-message').html(_Messages.getString('LoadingPage') + " " + mainJobStatus.page + " " + _Messages.getString('Of') + " " + mainJobStatus.totalPages); | |||
$('#notification-message').html(xssUtil.sanitizeHtml(_Messages.getString('LoadingPage') + " " + mainJobStatus.page + " " + _Messages.getString('Of') + " " + mainJobStatus.totalPages)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Idem, for use of xssUtil.setHtml(...)
given a jQuery object.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated
@@ -1242,13 +1242,13 @@ define([ 'common-ui/util/util', 'common-ui/util/timeutil', 'common-ui/util/forma | |||
case "QUEUED": | |||
case "WORKING": | |||
// Although we are hiding the screen latter, update the label anyway; | |||
$('#notification-message').html(_Messages.getString('LoadingPage') + " " + mainJobStatus.page + " " + _Messages.getString('Of') + " " + mainJobStatus.totalPages); | |||
$('#notification-message').html(xssUtil.sanitizeHtml(_Messages.getString('LoadingPage') + " " + mainJobStatus.page + " " + _Messages.getString('Of') + " " + mainJobStatus.totalPages)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Idem, for use of xssUtil.setHtml(...)
given a jQuery object.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated
New version of old PR, having unresolved comments: #864. |
3b8ea76
to
0e6eaa6
Compare
This comment has been minimized.
This comment has been minimized.
0e6eaa6
to
a05a08e
Compare
❌ Build failed in 3m 44sBuild command: mvn clean verify -B -e -Daudit -Djs.no.sandbox -pl core 👌 All tests passed! Tests run: 540, Failures: 0, Skipped: 3 Test Results Errors:Filtered log (click to expand)
ℹ️ This is an automatic message |
PPP-5353-XSS Findings For Pentaho-platform-plugin-reporting