ezimuel / zend-expressive-api

Web API skeleton using zend-expressive
59 stars 30 forks source link

RestDispatchTrait::handle security #9

Closed marcelto closed 6 years ago

marcelto commented 6 years ago

I would suggest explicitly defining which methods are allowed, either by a switch or a white list. The call to method_exists is very flexible and not necessarily insecure, however, I feel that it raises some concern in this context.

ezimuel commented 6 years ago

@marcelto the allowed methods are restricted by $request->getMethod(), no security issue here. Moreover, the allowed HTTP methods are specified in the routing section of the handler, giving a limited options of potential function calls by method_exists().

marcelto commented 6 years ago

Thanks @ezimuel. The \Zend\Diactoros\RequestTrait::validateMethod within zendframework/zend-diactoros doesn't seem overly restrictive and \Zend\Diactoros\RequestTrait::getMethod simply returns the property. Although the handler routing does look sufficiently restrictive provided the route method is not used directly without passing the $methods argument (which is optional). This seems fine as a certain level of coder responsibility should be expected. Thank you for taking the time to address my concern.