Skip to content

Commit

Permalink
Improved: Prevent URL parameters manipulation (OFBIZ-13147)
Browse files Browse the repository at this point in the history
The "JavaScriptEnabled=Y" and "&wt=javabin" references are weaknesses.
I temporarily put them in ControlFilter::doFilter to allow things (demo and
integration tests) to work for my test (only possible on a site w. domain IP),
ie not locally.

I think we can remove  "JavaScriptEnabled=Y". I put it there because we use it
in links at https://ofbiz.apache.org/ofbiz-demos.html. Maybe other places where
it's easy to remove w/o side effects. It's anyway an user preference, not
mandatory in query string.

I needed "&wt=javabin" for the Solr tests to pass. Sometimes ago I already faced
a such issue. And then put in place what's needed. ControlFilter::isSolrTest is
the solution by generalising this usage.

Conflicts handled by hand
  • Loading branch information
JacquesLeRoux committed Oct 24, 2024
1 parent 721e2e8 commit f5e55c4
Showing 1 changed file with 7 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,10 @@ public void init(FilterConfig filterConfig) throws ServletException {
}
}

private static boolean isSolrTest() {
return !GenericValue.getStackTraceAsString().contains("ControlFilterTests")
&& null == System.getProperty("SolrDispatchFilter");
}
@Override
public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain) throws IOException, ServletException {
HttpServletRequest httpRequest = (HttpServletRequest) request;
Expand Down Expand Up @@ -142,11 +146,9 @@ public void doFilter(ServletRequest request, ServletResponse response, FilterCha
String queryString = httpRequest.getQueryString();
if (queryString != null) {
queryString = URLDecoder.decode(queryString, "UTF-8");
// wt=javabin allows Solr tests, see https://cwiki.apache.org/confluence/display/solr/javabin
if (UtilValidate.isUrl(queryString)
|| !SecuredUpload.isValidText(queryString, Collections.emptyList())
&& !(queryString.contains("JavaScriptEnabled=Y")
|| queryString.contains("wt=javabin"))) {
&& isSolrTest()) {
Debug.logError("For security reason this URL is not accepted", module);
throw new RuntimeException("For security reason this URL is not accepted");
}
Expand All @@ -173,9 +175,8 @@ public void doFilter(ServletRequest request, ServletResponse response, FilterCha

GenericValue userLogin = (GenericValue) httpRequest.getSession().getAttribute("userLogin");
if (!LoginWorker.hasBasePermission(userLogin, httpRequest)) { // Allows UEL and FlexibleString (OFBIZ-12602)
if (!GenericValue.getStackTraceAsString().contains("ControlFilterTests")
&& null == System.getProperty("SolrDispatchFilter") // Allows Solr tests
&& SecurityUtil.containsFreemarkerInterpolation(httpRequest, httpResponse, requestUri)) {
if (isSolrTest() // Allows Solr tests
&& SecurityUtil.containsFreemarkerInterpolation(httpRequest, httpResponse, requestUri)) {
return;
}
}
Expand Down

0 comments on commit f5e55c4

Please sign in to comment.