Escape reflected request parameters in the sample servlets - #1129
Conversation
The client SDK sample war (ServiceConfigServlet, UserProfileServlet) and the policy evaluation demo servlet echoed request parameters (service name, user name, resource) into their HTML response unescaped - a reflected XSS in code people deploy as-is and copy from. Escape them with XMLUtils.escapeSpecialCharacters. The demo also imported the long-gone com.iplanet.am.util.XMLUtils, so it did not compile; it now uses com.sun.identity.shared.xml.XMLUtils. Closes CodeQL java/xss alerts OpenIdentityPlatform#112, OpenIdentityPlatform#113, OpenIdentityPlatform#114, OpenIdentityPlatform#115
vharseko
added
java
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| out.println("<h3>ServiceName:</h3> " + servicename); | ||
| out.println("<br><h3>Username:</h3> " + username); | ||
| // Request parameters are reflected into the page: escape them. | ||
| out.println("<h3>ServiceName:</h3> " + XMLUtils.escapeSpecialCharacters(servicename)); |
There was a problem hiding this comment.
4e7797c replaces XMLUtils.escapeSpecialCharacters here with commons-lang3 StringEscapeUtils.escapeHtml4, the escaper #1128 uses for the SAML1 POST target. CodeQL follows taint through escapeSpecialCharacters' char-by-char loop (the only escapers it treats as XSS sanitizers are methods named htmlEscape*); it has no model for escapeHtml4, so taint stops there.
| out.println("<br><h3>Username:</h3> " + username); | ||
| // Request parameters are reflected into the page: escape them. | ||
| out.println("<h3>ServiceName:</h3> " + XMLUtils.escapeSpecialCharacters(servicename)); | ||
| out.println("<br><h3>Username:</h3> " + XMLUtils.escapeSpecialCharacters(username)); |
There was a problem hiding this comment.
4e7797c replaces XMLUtils.escapeSpecialCharacters here with commons-lang3 StringEscapeUtils.escapeHtml4, the escaper #1128 uses for the SAML1 POST target. CodeQL follows taint through escapeSpecialCharacters' char-by-char loop (the only escapers it treats as XSS sanitizers are methods named htmlEscape*); it has no model for escapeHtml4, so taint stops there.
|
|
||
| out.println("<br><h3>Username:</h3> " + username); | ||
| // The user name is a request parameter reflected into the page: escape it. | ||
| out.println("<br><h3>Username:</h3> " + XMLUtils.escapeSpecialCharacters(username)); |
There was a problem hiding this comment.
4e7797c replaces XMLUtils.escapeSpecialCharacters here with commons-lang3 StringEscapeUtils.escapeHtml4, the escaper #1128 uses for the SAML1 POST target. CodeQL follows taint through escapeSpecialCharacters' char-by-char loop (the only escapers it treats as XSS sanitizers are methods named htmlEscape*); it has no model for escapeHtml4, so taint stops there.
CodeQL kept reporting java/xss on the escaped lines: it follows taint through XMLUtils.escapeSpecialCharacters' char-by-char loop. Use commons-lang3 StringEscapeUtils.escapeHtml4 instead, as OpenIdentityPlatform#1128 does for the SAML1 POST target; taint does not flow through it. The clientsdk sample war gains the commons-lang3 dependency (the shaded clientsdk jar does not bundle it), the policy demo drops its XMLUtils import.
3 participants
The client SDK sample war (
ServiceConfigServlet,UserProfileServlet) and the policy evaluation demo servlet (openam-distribution/.../demo/source/.../EvaluatePolicyServlet.java) echoed request parameters - service name, user name, resource - into their HTML response unescaped: a reflected XSS in code people deploy as-is and copy from.StringEscapeUtils.escapeHtml4, the escaper Validate ID-FF forward targets, FilesRepo identity names and SAML1 POST target #1128 uses for the SAML1 POST target. The sample war gains thecommons-lang3dependency (version managed, 3.20.0): the shaded clientsdk jar does not bundle it and ESAPI only pulls in commons-lang 2.6. ESAPI itself is declared in the sample war but has noESAPI.propertiesthere, so it is not used.com.iplanet.am.util.XMLUtilsand therefore did not compile; the import is gone (both reflected values now go throughescapeHtml4). Verified with a manualjavacagainst the openam-core classpath, where commons-lang3 already is (the demo is shipped as source and not built by Maven).openam-example-clientsdk-warcompiles. No unit tests: the sample modules have no test infrastructure and the change is mechanical escaping.CodeQL on this PR
The first revision escaped with
XMLUtils.escapeSpecialCharacters(clientsdk jar, no new dependency) and drew the samejava/xssfindings on the escaped lines (#495, #496, #497): CodeQL follows taint through that method's char-by-char loop, and the only escapers it treats as XSS sanitizers are methods namedhtmlEscape*. It has no model forescapeHtml4, so taint stops there. 4e7797c switches toescapeHtml4; the three threads are answered inline.Closes CodeQL
java/xssalerts #112, #113, #114, #115.