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,
}
// checkArgs refuses an argument that looks like an option and is not one
// of quarry's own.
//
// THE VULNERABILITY IT CLOSES. The file, tree, doc and history routes
// put a request's path and version segments into argv as positional
// arguments. zit parses any argument beginning with "-" as an option,
// and has no "--" end-of-options marker to stop it (danmestas/zit#206).
// 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.
func checkArgs(args []string) error {
for _, a := range args {
if !strings.HasPrefix(a, "-") || quarryFlags[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
}
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
}