java: false positive: javax.validation.constraints are not identified as input validation

[tpokki]

trafficstars

Description of the false positive

java.validation.constraints.* are not identified for input validation. For example in following example the id path param is considered to be insecure.

Example

import javax.validation.constraints.Max;
import javax.validation.constraints.Min;
import javax.validation.constraints.NotNull;
import javax.ws.rs.DefaultValue;
import javax.ws.rs.GET;
import javax.ws.rs.Path;
import javax.ws.rs.PathParam;
import javax.ws.rs.Produces;
import javax.ws.rs.core.MediaType;

@Path("/")
@Produces(MediaType.APPLICATION_JSON)
public interface API {
	@GET
	@Produces(MediaType.APPLICATION_JSON)
	@Path("/api/v1/thing/{id}")
        Something get(@NotNull @PathParam("id") @Min(1) @Max(30) @DefaultValue("1") Integer id);
}

[smowton]

Which particular query is considering that a dangerous value?

[tpokki]

I've seen false positives for

• java/sql-injection
• java/log-injection
• java/tainted-arithmetic

So far it seems that almost any query that checks on unsanitized user input.

[smowton]

Can you give a concrete example for sql-injection? That query has a default

  override predicate isSanitizer(DataFlow::Node node) {
    node.getType() instanceof PrimitiveType or
    node.getType() instanceof BoxedType or
    node.getType() instanceof NumberType
  }

That should immediately clean id not because of any annotation, but because it is of boxed primitive type.

[tpokki]

Sorry, I was a bit unclear. The above example with Integer id can cause java/tainted-arithmetic issue. While if the id would be String it can cause java/sql-injection.

[tpokki]

Created a small demo app in here.

Also noticed that the CodeQL does not even detect and report the flow thru ApiV1, as it does not have @Path annotation in the controller class, but only on interface. This looks like different issue/bug?

All endpoints in ApiV2 and ApiV3 are reported as vulnerable for all three aforementioned queries. Regardless of how the input validation is done (annotation, manual), or not done at all.

Are these CodeQL queries meant to detect if the input validation is done before the potentially hazardous code? Or just look for code patterns that are not best practice with possible input flows?

[smowton]

OK, so you're saying

1. Notice inherited @Path annotations when identifying sources
2. Notice that @Pattern may sanitise a string by ruling out certain characters
3. Notice that @Min and @Max may sanitise an integer

Is that right?

[tpokki]

Yes, that summaries my findings/questions so far in regards of jaxrs and javax.validations.

[alemanek]

I am also seeing this issue. Specifically with the "@Pattern" validation annotation not being seen as validation for a string input for the java/log-injection check.

Anyone have an idea on a workaround until this issue gets resolved?