Further soundness issue in LocalRequest/LocalResponse

[Qwaz]

Continued from #1312, Rocket v0.4.5.

Given that we make no use of this extension, such an extension is safe

I looked into the code again to see if the same APIs can be kept while removing the unsafe code. The more I look into the code, the more I believe we can't achieve the goal because the creation of LocalResponse<'c> is inherently unsound.

When handling requests, Rocket's dispatch() method takes &'r mut Request<'s> and generates Response<'r>. That is, Response instance is allowed to depend on values which live as long as Request instance.

When dispatching a request from LocalRequest, we create a LocalResponse<'c> from LocalRequest<'c> by extending the lifetime of LocalRequest reference behind Rc smart pointer to 'c with unsafe method. Here, 'c is the lifetime of Client instance (if I'm not misunderstanding anything).

This extension could be dangerous if the following conditions are true:

1. The actual lifetime of Request is shorter than the lifetime of Client.
2. A value that comes from Request is accessible after Request is dropped because it is annotated as 'c.

LocalResponse tries to prevent this situation by holding Rc<Request<'c>> with Response<'c>. However, this is not enough to prevent such scenarios to happen. I don't have a concrete proof of concept code like the one I attached in #1312 yet, but I believe the following code can pass Rust compiler's type check while performing use-after-free. Could you check the scenarios and share your thoughts?

A body with custom Drop implementation

According to RFC 1857, fields of structs are dropped in the same order as they are declared. It means that when dropping LocalResponse, _request: Rc<Request<'c>> is dropped before response: Response<'c>. If body field of Response, which has a type of Option<Body<Box<dyn io::Read + 'c>>>, contains a trait object with custom drop implementation, that drop implementation can trigger use-after-free if it accesses values that come from the original request.

Response::take_body()

Response::take_body() returns Option<Body<Box<dyn Read + 'c>>>. If we create LocalResponse, save the result of take_body(), and drop LocalRequest and LocalResponse, the underlying Request object is dropped but we still have access to the result of take_body() since it is annotated as 'c. Use-after-free can be triggered if this body object depend on values from Request instance.

[SergioBenitez]

This has been entirely reworked in master with a new strategy that I feel confident is sound. Please do let me know if you spot any issues!