package zitcli
import (
"fmt"
"strings"
)
// quarryFlags is every option string quarry itself passes to zit.
//
// It is a CLOSED SET, and that is the whole mechanism: quarry's flags
// are string literals in this package, while an attacker's arrive in a
// URL. At the argv layer the two are indistinguishable — both are just
// strings beginning with a dash — so the only thing that separates them
// is that one set can be written down in advance and the other cannot.
//
// Adding a flag to a zit invocation means adding it here. A missing
// entry fails that command loudly rather than silently, which is the
// direction this guard must fail in.
var quarryFlags = map[string]bool{
"--branch": true, "--brief": true, "--comment": true, "--event": true,
"--force": true, "--from": true, "--into": true, "--long": true,
"--mimetype": true, "--name": true, "--probe": true, "--propagate": true,
"--raw": true, "--reply-to": true, "--structural": true, "--time": true,
"--title": true, "--to": true, "--user": true,
"-R": true, "-c": true, "-m": true, "-n": true,
"--no-cache": true, "--limit": true, "--depth": true, "--out": true,
}
// quarryJoinedFlags is the smaller set quarry passes in zit's JOINED
// form, "--under=internal", where the part after the "=" is data the
// caller supplied.
//
// Separate from quarryFlags on purpose. A joined boolean is exactly
// what an injected URL segment looks like — "--no-cache=1" was the
// third row of the guard's own test — so only flags quarry actually
// spells this way are read for their name. The value needs no check
// of its own: it is one argv token bound to a flag name in this set,
// so it can no more become a second option than "--under internal"
// could.
var quarryJoinedFlags = map[string]bool{
"--under": true, "--since": true, "--verify": true, "--store": true,
}
// checkArgs refuses an argument that looks like an option and is not one
// of quarry's own, up to the end-of-options marker.
//
// THE VULNERABILITY IT CLOSED. The file, tree, doc and history routes
// put a request's path and version segments into argv as positional
// arguments. zit parsed any argument beginning with "-" as an option,
// and had no "--" end-of-options marker to stop it. So a URL segment
// WAS a flag:
//
// GET /api/v1/r/<repo>/file/trunk/--help -> 200, zit's help as file content
//
// Anonymous, on every repository, needing no capability. Found by jva,
// who filed it as a cosmetic 500 and then re-classified it when their
// adversarial subagent asked what was on the other end of the 500. I
// had investigated the same ticket hours earlier, found the missing
// "--", filed it upstream, and never asked what the segment BECOMES.
//
// Why here and not at each caller: a dozen methods take a user-supplied
// path, version, ref or name, and the next one added would not know to
// guard itself. This is the single place every invocation passes
// through, so a new call site is covered the day it is written.
//
// Why not simply reject every leading dash: quarry passes its own flags
// through the same door.
//
// WHAT THE MARKER CHANGED. zit 0.3.25 honours "--" in every verb, so
// the call sites that carry a URL segment now put one in front of it
// and the scan stops there: past the marker nothing is an option, and
// a name beginning with a dash — which is what the whole upstream fix
// exists to make reachable — is passed through rather than refused.
// The guard stays in front of it because the marker is a thing a call
// site remembers and this is a thing nobody has to: an invocation that
// forgets the marker is still refused rather than exploited.
func checkArgs(args []string) error {
for _, a := range args {
if a == "--" {
return nil
}
if !strings.HasPrefix(a, "-") || quarryFlags[a] || joinedFlag(a) {
continue
}
// A negative number is an operand, not an option: zit takes
// them for limits and offsets, and refusing them would break
// callers for no security gain.
if isNegativeNumber(a) {
continue
}
return fmt.Errorf("%w: refusing to pass %q to zit as an option", ErrUnsafeArgument, a)
}
return nil
}
// joinedFlag reports an argument of the form "--<flag>=<value>" whose
// flag is one quarry passes that way.
//
// This is what broke every subdirectory page on the forge. The guard
// compared the WHOLE token, so "--under=internal" was not "--under",
// the tree listing was refused, and tree.list turned the refusal into
// a 404 — on every repository, for every directory, while the suite
// stayed green because nothing listed a directory through a real zit.
// TestListingASubdirectory is that missing row.
func joinedFlag(a string) bool {
if !strings.HasPrefix(a, "--") {
return false
}
name, _, ok := strings.Cut(a, "=")
return ok && quarryJoinedFlags[name]
}
func isNegativeNumber(s string) bool {
if len(s) < 2 || s[0] != '-' {
return false
}
for _, r := range s[1:] {
if r < '0' || r > '9' {
return false
}
}
return true
}