On 11/8/2017 12:01 AM, Junio C Hamano wrote:
Jonathan Tan <jonathanta...@google.com> writes:
Having said that, though, it might be safer to still introduce one, and
relax it later if necessary - it is much easier to relax a constraint
than to increase one.
It would also be more error prone to have such a long switch ()
statement, each of whose case arm needs to be carefully looked at.
While protection against attacks over the wire against the process
that receives the request is necessary and doing the quoting right
at this layer is one valuable component of it, we would need to be
careful about what features we allow the other side to request.
For example, an innocent-looking use of get_oid_with_context() can
trigger an expensive operation, e.g. "master^{/sekritCodeName}", may
not just waste resources but also may reveal the presence of an
object that we might not want to leak to a stranger. Limiting such
an abuse must sit at a lot higher layer than a byte-by-byte check
over the request like the code does.
Right. I could see adding another server-side variable in the
spirit of the existing "uploadpack.allow*" variables.
My main concern at this point has been avoiding injections.
Jeff
