codeconsole commented on PR #16786:
URL: https://github.com/apache/grails-core/pull/16786#issuecomment-6506736147
You're right that a task must not keep a reference into the request once the
container owns it again, and detaching only `lookup()` doesn't guarantee that.
A task that took `request`, `params` or the web request while the request was
live keeps it. With `discardFacades=false`, or a container that reuses its
request objects, that reference then reads the next user's request. That's the
CVE shape, and the copy's indirection does protect against it.
The copy brings its own exposure, though. I checked 3847d6f006 against
embedded Tomcat 11: a multipart POST that a security filter rejected with 403,
with a task started inside that filter (an async audit of the denial, say). As
the request completes, the copy calls `getParameterMap()`, and Tomcat parses
the whole multipart body for a client that was never authenticated, writing its
5 MB file part to disk. #16149 turned the hidden-method filter off by default
to stop exactly that parse before authentication. The copy also reads up to 2
MB of any other body nothing read, and it still leaves the response pointing at
the recycled facade.
What gives the safety is the indirection, not the copy. So: give the task a
thin `HttpServletRequestWrapper` and `HttpServletResponseWrapper` around the
live request and response. When the request completes, swap their delegates, in
that one place, for ones that throw `IllegalStateException("the request has
completed")` on every call, and mark the task's web request completed, so
`lookup()` and binding treat the task as running outside a request. That keeps
the lifecycle centralized, fails closed for any reference a task captured,
covers the response too, and reads nothing from the request as it completes.
It's also a real `ServletRequestWrapper`, so `getNativeRequest` keeps working
while the request is live. The one thing it gives up is a task reading request
data after completion, which Spring doesn't allow either. A task that needs
that data should take the values while the request is live.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
