mojarra icon indicating copy to clipboard operation
mojarra copied to clipboard

Partial rendering: insufficient CDATA encoding (XSS)

Open cnsgithub opened this issue 6 years ago • 12 comments

1) Environment

  • Mojarra version: 2.3.2

2) Expected behavior

Using CDATA end tag ]]> within submitted ajax strings must not break things when included in partial response.

3) Actual behavior

Client side partial response processing gets aborted. This issue may lead to partial response (XML) injection/XSS as seen in https://github.com/primefaces/primefaces/issues/3623 and reported in https://github.com/primefaces/primefaces/issues/3629 also. This issue was also mentioned in https://github.com/javaserverfaces/mojarra/issues/3042 but has not yet been addressed.

4) XHTML

        <h:form id="form">
            <h:commandButton value="Submit" action="#{bean.submit}">
                <f:ajax event="click" />
            </h:commandButton>
            <h:panelGroup id="panel" />
        </h:form>

5) Bean

@ManagedBean
public class Bean {

    public void submit() {
        FacesContext ctx = FacesContext.getCurrentInstance();
        ExternalContext extContext = ctx.getExternalContext();
        if (ctx.getPartialViewContext().isAjaxRequest()) {
            try {
                extContext.setResponseContentType("text/xml");
                extContext.addResponseHeader("Cache-Control", "no-cache");
                PartialResponseWriter writer = ctx.getPartialViewContext().getPartialResponseWriter();
                writer.startDocument();
                writer.startUpdate("form:panel");
                writer.startElement("span", null);
                writer.writeAttribute("id", "form:panel", null);
                writer.startElement("script", null);
                writer.writeText("alert('User input: " + escapeJavascript(getUserInput()) + "')", null);
                writer.endElement("script");
                writer.endElement("span");
                writer.endUpdate();
                writer.endDocument();
                writer.flush();
                ctx.responseComplete();
            }
            catch (Exception e) {
                throw new FacesException(e);
            }
        }
    }
    
    private String getUserInput() {
        return "foo]]>bar";
    }
    
    private String escapeJavascript(String string) {
        // would use OWASP javascript escaping for example
        // no need to escape ]]> in javascript strings, so it remains unchanged
        return string;
    }
}

5) Misc

  • Possible solution: In PartialResponseWriter/HtmlResponseWriter.writeText replace ]]> with ]]]]><![CDATA[> when dontEscape && writingCdata == true.
  • Further readings: https://stackoverflow.com/questions/223652/is-there-a-way-to-escape-a-cdata-end-token-in-xml and https://www.owasp.org/index.php/Testing_for_XML_Injection_(OTG-INPVAL-008)

cnsgithub avatar May 04 '18 09:05 cnsgithub

As this might be a security risk, it should be addressed with higher priority. Just ignoring it again seems not to be the right solution.

Btw, you may have a look at owasp's solution to get an idea how to fix it: https://github.com/OWASP/owasp-java-encoder/blob/master/core/src/main/java/org/owasp/encoder/CDATAEncoder.java

cnsgithub avatar Jun 20 '18 05:06 cnsgithub

This is quite critical for us as well. Several composite components and PrimeFaces components have CDATA sections with scripts and when such components are being re-rendered due to an ajax partial update everything breaks. We are on Mojarra 2.3.5.SP2 which comes with the very latest WildFly 14.0.1

marcodelpercio avatar Oct 08 '18 13:10 marcodelpercio

Please follow https://github.com/eclipse-ee4j/mojarra/issues/4392 at EE4J since mojarra has moved there.

cnsgithub avatar Nov 21 '18 10:11 cnsgithub

This is quite critical for us as well. Several composite components and PrimeFaces components have CDATA sections with scripts and when such components are being re-rendered due to an ajax partial update everything breaks. We are on Mojarra 2.3.5.SP2 which comes with the very latest WildFly 14.0.1

Hi @marcodelpercio,

I have the same problem with Wildfly 14 that implements jsf 2.3.5, have you found a solution?

Thanks

trinidadmarcelo avatar Apr 02 '19 20:04 trinidadmarcelo

@trinidadmarcelo Would it be an option to replace the JSF implementation in WildFly modules with MyFaces? Things seem to get fixed faster there... And it does not suffer from this issue at all.

cnsgithub avatar Apr 03 '19 04:04 cnsgithub

@trinidadmarcelo I do actually have a workaround. It's a simple WebFilter which intercepts CDATA comments inside AJAX requests/partial requests and replaces the wrong cdata comment with a working one.

It's a bit sad that this issue hasn't been fixed in the last 5 months. I am still hoping for an official solution from the Mojarra team.

@cnsgithub Have you actually tried going MyFaces in WildFly? Did it work?

CDataFix.java.txt

marcodelpercio avatar Apr 03 '19 07:04 marcodelpercio

@cnsgithub Have you actually tried going MyFaces in WildFly? Did it work?

Not yet, sorry. Thanks for sharing your workaround with the community.

cnsgithub avatar Apr 03 '19 07:04 cnsgithub

@trinidadmarcelo Would it be an option to replace the JSF implementation in WildFly modules with MyFaces? Things seem to get fixed faster there... And it does not suffer from this issue at all.

I have replaced with the jsf impl 2.3.2 inside modules of Wildfly 14, the jsf spec also must be replaced and it works, the problem is since jsf impl 2.3.5 that implements since Wildfly 14

trinidadmarcelo avatar Apr 03 '19 15:04 trinidadmarcelo

@trinidadmarcelo I do actually have a workaround. It's a simple WebFilter which intercepts CDATA comments inside AJAX requests/partial requests and replaces the wrong cdata comment with a working one.

It's a bit sad that this issue hasn't been fixed in the last 5 months. I am still hoping for an official solution from the Mojarra team.

@cnsgithub Have you actually tried going MyFaces in WildFly? Did it work?

CDataFix.java.txt

@marcodelpercio as a quick solution using Wildfly 14 with no modifications in his jsf modules, I replaced all javascript CDATA with the XHTML escape codes, hope Mojarra team fix it asap.

trinidadmarcelo avatar Apr 03 '19 15:04 trinidadmarcelo

@trinidadmarcelo I do actually have a workaround. It's a simple WebFilter which intercepts CDATA comments inside AJAX requests/partial requests and replaces the wrong cdata comment with a working one.

It's a bit sad that this issue hasn't been fixed in the last 5 months. I am still hoping for an official solution from the Mojarra team.

@cnsgithub Have you actually tried going MyFaces in WildFly? Did it work?

CDataFix.java.txt

I installed MyFaces sucessfully, using multi-jsf-installer bundled in wildfly sources. But it just create the folder if using linux, I did it and after I could use on Windows. I did it with version 16. I attached the zip here (it contains mojarra 2.2.16 and myfaces 2.3.3 as modules) , you just extract it in modules root folder. And in standalone.xml you modify the line of jsf this way <subsystem xmlns="urn:jboss:domain:jsf:1.1" default-jsf-impl-slot="myfaces-2.3.3"/> com.zip

erickdeoliveiraleal avatar Apr 03 '19 21:04 erickdeoliveiraleal

It is quite similar to https://github.com/eclipse-ee4j/mojarra/issues/4488. Unfortunately mojarra not just buggy, but after examining the implementation, the code quality seems to be very poor also.

abelet avatar Apr 06 '19 08:04 abelet

Since this is fixed in the 2.3 branch, could this be backported to 2.2?

kguelzau avatar Aug 20 '20 10:08 kguelzau