Java: Add sources for Jax-RS filters #6478
Conversation
javaGenerated file changes for java
- Java extensions,"``javax.*``, ``jakarta.*``",22,540,27,,,,,1,1,2
+ Java extensions,"``javax.*``, ``jakarta.*``",36,546,27,,,,,1,1,2
- Totals,,84,2711,398,13,6,6,107,33,1,66
+ Totals,,98,2717,398,13,6,6,107,33,1,66
+ jakarta.ws.rs.container,,7,,,,,,,,,,,,,,,,,,,7,,
- jakarta.ws.rs.core,2,,143,,,,,,,,,,,,,,,2,,,,88,55
+ jakarta.ws.rs.core,2,,146,,,,,,,,,,,,,,,2,,,,91,55
+ javax.ws.rs.container,,7,,,,,,,,,,,,,,,,,,,7,,
- javax.ws.rs.core,3,,143,,,,1,,,,,,,,,,,2,,,,88,55
+ javax.ws.rs.core,3,,146,,,,1,,,,,,,,,,,2,,,,91,55 |
| "javax.ws.rs.core;UriInfo;true;getPathParameters;;;Argument[-1];ReturnValue;taint", | ||
| "javax.ws.rs.core;UriInfo;true;getPathSegments;;;Argument[-1];ReturnValue;taint", | ||
| "javax.ws.rs.core;UriInfo;true;getQueryParameters;;;Argument[-1];ReturnValue;taint", | ||
| "javax.ws.rs.core;UriInfo;true;getRequestUri;;;Argument[-1];ReturnValue;taint", | ||
| "javax.ws.rs.core;UriInfo;true;getRequestUriBuilder;;;Argument[-1];ReturnValue;taint", | ||
| "jakarta.ws.rs.core;UriInfo;true;getAbsolutePath;;;Argument[-1];ReturnValue;taint", | ||
| "jakarta.ws.rs.core;UriInfo;true;getAbsolutePathBuilder;;;Argument[-1];ReturnValue;taint", | ||
| "jakarta.ws.rs.core;UriInfo;true;getPath;;;Argument[-1];ReturnValue;taint", |
There was a problem hiding this comment.
I wonder if calls to relativize should mark it's return value as tainted as it may include path elements
| private class ContainerRequestContextModel extends SourceModelCsv { | ||
| override predicate row(string s) { | ||
| s = | ||
| ["javax", "jakarta"] + ".ws.rs.container;ContainerRequestContext;true;" + |
| [ | ||
| "getAcceptableLanguages", "getAcceptableMediaTypes", "getCookies", "getEntityStream", | ||
| "getHeaders", "getHeaderString", "getUriInfo" | ||
| ] + ";;;ReturnValue;remote" |
There was a problem hiding this comment.
Not sure how feasible that is right now but I wonder if the context should also be treated like a Map to ensure the setProperty and getProperty calls propagate taint.
There was a problem hiding this comment.
In common with my approach to JSON-related libs I didn't bother to distinguish them because the use case is deserialization -> read or write -> serialization, where we only care about a single monolithic concept of taint, as opposed to map-writes followed by map-reads as with a general-purpose data structure. If we catch people actually using these like general-purpose data structures in the wild though, then we should do the extra work and increase the fidelity of our modelling as you suggest.
There was a problem hiding this comment.
Generally agree. In this particular case, I thought that the context properties are actually the designated way to pass data among the filters and to the content producers (see also https://abhirockzz.wordpress.com/2016/03/16/sharing-data-between-jax-rs-filters/ ). But this may be better modelled as part of a query than here.
There was a problem hiding this comment.
Ah, interesting, hadn't seen that done. To support that case we would need to model the control-flow from one filter to another, since that is implemented in opaque framework code.
javaGenerated file changes for java
- Java extensions,"``javax.*``, ``jakarta.*``",22,540,27,,,,,1,1,2
+ Java extensions,"``javax.*``, ``jakarta.*``",36,552,27,,,,,1,1,2
- Totals,,84,2711,398,13,6,6,107,33,1,66
+ Totals,,98,2723,398,13,6,6,107,33,1,66
+ jakarta.ws.rs.container,,7,,,,,,,,,,,,,,,,,,,7,,
- jakarta.ws.rs.core,2,,143,,,,,,,,,,,,,,,2,,,,88,55
+ jakarta.ws.rs.core,2,,149,,,,,,,,,,,,,,,2,,,,94,55
+ javax.ws.rs.container,,7,,,,,,,,,,,,,,,,,,,7,,
- javax.ws.rs.core,3,,143,,,,1,,,,,,,,,,,2,,,,88,55
+ javax.ws.rs.core,3,,149,,,,1,,,,,,,,,,,2,,,,94,55 |
javaGenerated file changes for java
- Java extensions,"``javax.*``, ``jakarta.*``",22,540,27,,,,,1,1,2
+ Java extensions,"``javax.*``, ``jakarta.*``",40,552,27,,,,,1,1,2
- Totals,,84,2711,398,13,6,6,107,33,1,66
+ Totals,,102,2723,398,13,6,6,107,33,1,66
+ jakarta.ws.rs.container,,9,,,,,,,,,,,,,,,,,,,9,,
- jakarta.ws.rs.core,2,,143,,,,,,,,,,,,,,,2,,,,88,55
+ jakarta.ws.rs.core,2,,149,,,,,,,,,,,,,,,2,,,,94,55
+ javax.ws.rs.container,,9,,,,,,,,,,,,,,,,,,,9,,
- javax.ws.rs.core,3,,143,,,,1,,,,,,,,,,,2,,,,88,55
+ javax.ws.rs.core,3,,149,,,,1,,,,,,,,,,,2,,,,94,55 |
smowton
force-pushed
the
smowton/feature/jax-rs-request-filters
branch
from
0e9ab52 to
95046b9
Compare
| s = | ||
| ["javax", "jakarta"] + ".ws.rs.container;ContainerRequestContext;true;" + | ||
| [ | ||
| "getAcceptableLanguages", "getAcceptableMediaTypes", "getCookies", "getEntityStream", |
There was a problem hiding this comment.
Can Locale contain arbitrary taint?
There was a problem hiding this comment.
As it turns out yes -- the docs for Locale specify what the parameters should be (e.g. language - An ISO 639 alpha-2 or alpha-3 language code), but it doesn't actually check that that's the case. I forget where exactly but I think @atorralba wrote a little test case recently showing attack code from an HTTP Accept header making it through unaltered to Locale.toString.
This also adds some missing UriInfo taint propagators I noticed while doing this.