Pushing a valid but empty pack (32 bytes: PACK header, object count 0, checksum) together with a ref update to a nonexistent oid gets accepted. The server answers ok, the ref is published to the WAL, and from that point on every clone/fetch that walks the ref dies with missing object.
Found this while reading the receive-pack handler, then confirmed against a local server.
Repro (raw HTTP, any write credential):
POST /{o}/{r}.git/git-receive-pack, command area: <40 zero bytes> <any 40-hex oid> refs/heads/ghost\0report-status, then flush
- body: a minimal empty pack, 32 bytes total (
PACK + version + count=0 + sha1 trailer)
- server reports
ok refs/heads/ghost; git ls-remote lists it, git clone fails on it
Two things combine here.
First, the connectivity check only runs when a pack was actually ingested:
if let Ok(Some(_)) = &ingest { // Ok(None) (zero-object pack) skips the whole block
https://github.com/tobi/walgit/blob/6d8fa54/crates/walgit-server/src/smart.rs#L1192-L1218
ingest_pack legitimately returns Ok(None) for an empty pack and git index-pack accepts it happily, so check_connectivity_async never runs for these pushes.
Second, verify_txn on the publish side only compares old_oid for the optimistic lock and never checks that new_oid exists anywhere:
https://github.com/tobi/walgit/blob/6d8fa54/crates/walgit-wal/src/publish.rs#L329-L363
So there is no backstop either.
Fix direction: run the existence/connectivity check whenever any update has a non-zero new_oid, not only when a pack was ingested. For Ok(None) pushes the objects are supposed to be local already, so it's a handful of index lookups and the malicious case fails fast. Alternatively (or additionally) a has_object check on non-zero new_oid inside verify_txn as a last-line defense.
Happy to put up a patch with a regression test if this sounds right — something like push_with_empty_pack_cannot_create_ref_to_missing_object in tests/e2e.rs.
Pushing a valid but empty pack (32 bytes:
PACKheader, object count 0, checksum) together with a ref update to a nonexistent oid gets accepted. The server answersok, the ref is published to the WAL, and from that point on every clone/fetch that walks the ref dies withmissing object.Found this while reading the receive-pack handler, then confirmed against a local server.
Repro (raw HTTP, any write credential):
POST /{o}/{r}.git/git-receive-pack, command area:<40 zero bytes> <any 40-hex oid> refs/heads/ghost\0report-status, then flushPACK+ version + count=0 + sha1 trailer)ok refs/heads/ghost;git ls-remotelists it,git clonefails on itTwo things combine here.
First, the connectivity check only runs when a pack was actually ingested:
https://github.com/tobi/walgit/blob/6d8fa54/crates/walgit-server/src/smart.rs#L1192-L1218
ingest_packlegitimately returnsOk(None)for an empty pack andgit index-packaccepts it happily, socheck_connectivity_asyncnever runs for these pushes.Second,
verify_txnon the publish side only comparesold_oidfor the optimistic lock and never checks thatnew_oidexists anywhere:https://github.com/tobi/walgit/blob/6d8fa54/crates/walgit-wal/src/publish.rs#L329-L363
So there is no backstop either.
Fix direction: run the existence/connectivity check whenever any update has a non-zero
new_oid, not only when a pack was ingested. ForOk(None)pushes the objects are supposed to be local already, so it's a handful of index lookups and the malicious case fails fast. Alternatively (or additionally) ahas_objectcheck on non-zeronew_oidinsideverify_txnas a last-line defense.Happy to put up a patch with a regression test if this sounds right — something like
push_with_empty_pack_cannot_create_ref_to_missing_objectintests/e2e.rs.