Skip to content
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

OC-17705:Fix XSS finding in report #3516

Open
wants to merge 1 commit into
base: 3.17.x
Choose a base branch
from

Conversation

Taoli2018
Copy link
Contributor

No description provided.

escapedStr= escapedStr.replaceAll("<script>", "");
escapedStr= escapedStr.replaceAll("</script>", "");
escapedStr= escapedStr.replaceAll("<style>", "");
escapedStr= escapedStr.replaceAll("</style>", "");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure we need to replace these tags if you are escaping javascript in the line below

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general, this seems like a big change and I am wary of the consequences of this change. I think we should limit the fix to the affected code by escaping javascript in the jsp that is displaying the query parameters. E.g. using a c:out tag will automatically escape the values.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that this'll be a big change if fix like this at code level, in this reported case, we use one 3rd party technology ---Jmesa dynamic tables (jmesa-2.4.2-oc.jar), so all page html code (include javascripts, css) are dynamically generated by the 3rd party library with the HTTP Request as one of the input, for example, in java class file ListStudySubjectsServlet.java, under the method createTable() will use below line to create table html:

String findSubjectsHtml = factory.createTable(request, response).render();

There is no place to use c:out tag to intercept the output of the above method,
I will do more research at Nginx content security policy setting, hope can find solution by modification of nginx configuration

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants