Fix 70208: Strengthen app context comparison logic

[Chenjp]

Strip Path Param and normalize it before context-path matching, may not perfect but do more.

[markt-asf]

I can see the benefit of normalization but I'm not convinced path parameter stripping is necessary. The context path part of the resulting normalized string will have been provided by the application so there is no reason for path parameters to be present.

[Chenjp]

I can see the benefit of normalization but I'm not convinced path parameter stripping is necessary. The context path part of the resulting normalized string will have been provided by the application so there is no reason for path parameters to be present.

Following story came from my partner cyb@zstu.edu.cn closed report, and we have talk it further:

Consider the possibility of user-input relative uri path, e.g., wordpress-in-java ("/wp") author's publish an article with following metadata:

<!--Malicious author adds new article -->
<article id="1024" logo_relative_uri="/images/..;/../public/images/logo.png" raw_relative_uri="/public/docs/..;/../../wp/profile;jsessionid=$injected_session_id" title="Tomcat encodeURL / encodeRedirectURL weakness"/>

Server-side saves his article submission, failed to recognize the risks of insufficiently sanitized relative URI paths - most applications generally assume that the relative uri paths are safe.

When another admin / public user visit /wp/articles/1024.

String logoRelativeUri="/images/..;/..;/public/images/logo.png";
...
<!--img node: Zero-click ? -->
<img src="<%= response.encodeURL(application.getContextPath() + logoRelativeUri) %>" alt="Loading"/>
...

We think the present of path parameters in relative path is possible.
And necessary to his resubmit?

[markt-asf]

This is a change to harden against (potentially) vulnerable web applications that are failing to validate/sanitise untrusted input. We need to weigh the benefits it brings to those broken applications against the cost it adds to applications that already do things properly and don't need this hardening.

Broken applications only benefit if the application doesn't use cookies for session tracking. Are there (m)any of those these days?

The cost is incurred by every application that uses the CsrfPreventionFilter.

The cost of the path parameter stripping is minimal (compared to existing CsrfPreventionFilter processing) assuming there are no path parameters. Normalization is the potentially more expensive operation and the one that is more likely to be used.

Given the above, the there is much less of a reason to skip the path parameter processing that I first thought.

I will note that the fix could use the existing stripPathParams function and that the context path comparison can be simplified because getContextPath() will never return a value that ends with a "/".

I'll apply a variation of this patch shortly.

[markt-asf]

Variation applied.

[Chenjp]

Variation applied.

Super quick than ever.