Java: add query to detect web.xml auth bypass through verb tampering

[porcupineyhairs]

This query detects the case where an auth-constraint is present for a particular HTTP verb but does not include some other verb.

The most common example for this case is CVE-2018-1306, the Apache Pluto 3.0.0 RCE.

Consider the config as shown below

  <security-constraint>
    <web-resource-collection>
      <web-resource-name>portal</web-resource-name>
      <url-pattern>/portal</url-pattern>
      <url-pattern>/portal/*</url-pattern>
      <http-method>GET</http-method>
      <http-method>POST</http-method>
      <http-method>PUT</http-method>
    </web-resource-collection>
    <auth-constraint>
      <role-name>pluto</role-name>
    </auth-constraint>
  </security-constraint>

In this case, the auth-constraint check for GET,POST and PUT methods but not for the HEAD method. Essentially resulting in a full authentication bypass and eventually RCE.

[ghost]

The output of this query against Apache Pluto 3.0.0 can be found at this link. Please note the query runs against my fork of Pluto as I couldn't get LGTM to run this against version 3.0.0 isntead of the current master.

[ghost]

@Marcono1234

I have squashed the commits and included some suggestions from the review.

RE: "verb" vs "methods": I can see that the standard calls it a method but the attack is called "HTTP Verb tampering". Hence, to avoid confusion, I have simply included a minor change to the initial line. So that it is now HTTP verbs(or methods)

[luchua-bc]

This is an interesting area. As not all Servlet implementations take all HTTP verbs, for example, a lot of servlets only take the GET and/or POST verb while denying the rest. E.g.

   public void doPost(HttpServletRequest request, HttpServletResponse response) {
        throw new HTTPException(HttpServletResponse.SC_METHOD_NOT_ALLOWED);
    }

    public void doDelete(HttpServletRequest request, HttpServletResponse response) {
        throw new HTTPException(HttpServletResponse.SC_METHOD_NOT_ALLOWED);
    }

    public void doPut(HttpServletRequest req, HttpServletResponse res) {
        throw new HTTPException(HttpServletResponse.SC_METHOD_NOT_ALLOWED);
    }

    public void doInfo(HttpServletRequest req, HttpServletResponse res) {
        throw new HTTPException(HttpServletResponse.SC_METHOD_NOT_ALLOWED);
    }

    public void doHead(HttpServletRequest request, HttpServletResponse response) {
        throw new HTTPException(HttpServletResponse.SC_METHOD_NOT_ALLOWED);
    }

    public void doOptions(HttpServletRequest req, HttpServletResponse res) {
        throw new HTTPException(HttpServletResponse.SC_METHOD_NOT_ALLOWED);
    }

To reduce false positives, will it be a good idea to at least amend the query to check that the Java code does handle additional verbs? Thanks.

[Marcono1234]

To add to @luchua-bc's comment by default you have to override the HttpServlet methods to make your application support them (except doGet which when overridden then also supports doHead unless that is overridden as well).

[smowton]

@porcupineyhairs as per comments above, I'd expect a very high false-positive rate from this because many of the methods are very uncommon (e.g. DELETE, PATCH, CONNECT are commonly left in their default reject-bad-method state).

In order from least to most false positives (and true positives of course), you could go for:

• GET is checked, but HEAD is not
• Any method is checked but [GET, HEAD] is not (FP if only POST/PUT requests require auth? Might apply to some APIs with public read-only / authenticated read-write access)
• Any method is checked but [GET, HEAD, POST] is not
• Any method is checked but [GET, HEAD, POST, PUT] is not

The later ones could be improved by looking for a doPost or other web frameworks' equivalent handlers for evidence that POST or PUT aren't just rejected.

Let me know which way you want to go and I'll start an evaluation.

[smowton]

Also am I right thinking there's no bounty application associated with this query at present?

[ghost]

@smowton Your ideas woud reduce the FP's. but I am still concerned with the number of results. Hence, I am keeping this one on hold for a while. Please give me some time to see what can be done to reduce the FP's here.

Also, this was meant to be a bug bounty submission but now that I am keeping it on hold, I am not opening a new issue.