On 2024-08-01 at 06:14:17, Jeff King wrote: > ...this is doing that loosening for upload-pack by default. I'm not sure > if that is OK or not. My mental model has remained "it is OK to run > upload-pack on an untrusted repository", but it would make sense to get > input from folks who looked at this in the past, like Dscho, and/or to > reassess the threat model from scratch. > > In particular I did not follow all of the potential issues with linked > local files. Are we good now after other fixes (in which case this patch > is OK)? Are we good only for non-local clones (so this patch is OK only > combined with a fix for clone to check ownership for --local mode)? Or > are there still problems if an attacker controls the repo paths, in > which case upload-pack should remain conservative? I think we already have such a patch. In clone, clone_local either has option_shared (in which case we simply refer to the other repository via an alternates file and don't touch it in any way), or it calls copy_or_link_directory, which in turn calls die_upon_dubious_ownership. The other case, where is_local is not set (and thus clone_local is not called), calls transport_fetch_refs, which either calls fetch_refs_via_pack or fetch_refs_via_bundle, both of which I assume actually make a git-upload-pack call. One related topic that is potentially interesting as well is whether `git bundle create` also offers the same security guarantees as `git upload-pack` in that it can be safely run on an untrusted repository. Either way, we may want to document that. -- brian m. carlson (they/them or he/him) Toronto, Ontario, CA
Attachment:
signature.asc
Description: PGP signature
