Java: Review changes for https://github.com/github/codeql/pull/3653 #1
Conversation
| @@ -377,34 +377,19 @@ private predicate taintPreservingQualifierToMethod(Method m) { | |||
| or | |||
| m = any(ProtobufMessageLite p).getAGetterMethod() | |||
| or | |||
| m instanceof MapMethod and | |||
| ( | |||
| m.getName().regexpMatch("get|entrySet|keySet|values") | |||
There was a problem hiding this comment.
Just noting that keySet is not covered in ContainerFlow (unlike the others). However, I believe this is fine for Spring, because a Map type parameter to a @RequestMapping method will be unlikely to contain tainted keys, and ContainerFlow only taints Maps via the value not the key parameter.
(ref: https://docs.spring.io/spring/docs/current/spring-framework-reference/web.html#mvc-ann-arguments)
There was a problem hiding this comment.
keySet is excluded on purpose as we currently otherwise won't be able to distinguish keys and values in a tainted map. If at some point tainted keys become important to track, then we'll most likely attempt this through a more precise modelling via container-content-as-field-flow.
| ( | ||
| m.getName().regexpMatch("get|toArray|subList|spliterator|set|iterator|listIterator") or | ||
| (m.getName().regexpMatch("remove") and not m.getReturnType() instanceof BooleanType) | ||
| ) |
There was a problem hiding this comment.
spliterator, set and listIterator are not covered by ContainerFlow.
…aflow-tests Small fixups to your PR to my PR
Rewrite XQuery injection to use an additional taint step instead of multiple configurations
No description provided.