diff --git a/crocmobile/errors_test.go b/crocmobile/errors_test.go index 054ff97c..fc6c7be9 100644 --- a/crocmobile/errors_test.go +++ b/crocmobile/errors_test.go @@ -9,6 +9,7 @@ import ( "net" "os" "path/filepath" + "slices" "strconv" "strings" "sync" @@ -256,3 +257,42 @@ func TestStartErrorsCarryPayload(t *testing.T) { t.Error("startError(nil) != nil") } } + +func FuzzClassify(f *testing.F) { + for _, s := range []string{ + "pake not successful: peer uses unsupported PAKE protocol version 0", + "peer error: refusing files", "refused files", "room full", "bad password", + "could not connect to relay", "transfer disconnected", "unexpected EOF", "", + } { + f.Add(s) + } + codes := []string{ + codePeerVersion, codeRelayUnreachable, codeNoPublicRelay, codeDeclined, codeRefused, + codeRoomFull, codeBadPassword, codeWrongCode, codeCancelled, codeConnectionLost, + codeStoreLimit, codeStoreExpired, codeInvalidSetting, codeOther, + } + f.Fuzz(func(t *testing.T, msg string) { + err := errors.New(msg) + got := classify(err) + if !slices.Contains(codes, got) { + t.Fatalf("classify(%q) = %q, not a known failure code", msg, got) + } + if classify(err) != got { + t.Fatalf("classify(%q) is not deterministic", msg) + } + // Wrapping must not change the code: the app relies on errors.As/Is and + // on text matching seeing through croc's own wrapping. + if w := classify(fmt.Errorf("wrapped: %w", err)); w != got && w != codeCancelled { + t.Fatalf("classify(wrapped %q) = %s, want %s", msg, w, got) + } + // A coded error carries its code regardless of its text. + if c := classify(&codedError{codeInvalidSetting, err}); c != codeInvalidSetting { + t.Fatalf("classify(coded, %q) = %s, want %s", msg, c, codeInvalidSetting) + } + // The failure payload must round-trip through JSON for any message. + var f failure + if jerr := json.Unmarshal([]byte(errorJSON(err)), &f); jerr != nil || f.Code != got { + t.Fatalf("errorJSON(%q) decoded to %+v, %v; want code %s", msg, f, jerr, got) + } + }) +} diff --git a/crocmobile/network.go b/crocmobile/network.go index 575d7bd8..384ab159 100644 --- a/crocmobile/network.go +++ b/crocmobile/network.go @@ -32,7 +32,7 @@ func relayPorts(o *Options) ([]string, error) { if n == 0 { n = DefaultTransfers } - if base < 1 || n < 1 || base+n > 65535 { + if base < 1 || n < 1 || base > 65535-n { return nil, invalidSetting("ports %d to %d are out of range", base, base+n) } ports := make([]string, n+1) diff --git a/crocmobile/network_test.go b/crocmobile/network_test.go index f5d80a2a..a1e68e33 100644 --- a/crocmobile/network_test.go +++ b/crocmobile/network_test.go @@ -3,10 +3,14 @@ package crocmobile import ( "encoding/json" "io" + "math" "net" "net/http" "path/filepath" "reflect" + "regexp" + "slices" + "strconv" "strings" "sync/atomic" "testing" @@ -283,3 +287,103 @@ func codeOf(err error) string { } return classify(err) } + +func FuzzRelayPorts(f *testing.F) { + f.Add(0, 0) + f.Add(9100, 2) + f.Add(50000, 1) + f.Add(65533, 2) + f.Add(65535, 0) + f.Add(-1, 0) + f.Add(0, -1) + f.Add(math.MaxInt, 1) + f.Add(1, math.MaxInt) + f.Fuzz(func(t *testing.T, base, transfers int) { + o := Options{BasePort: base, Transfers: transfers} + ports, err := relayPorts(&o) + if err != nil { + if codeOf(err) != codeInvalidSetting { + t.Fatalf("relayPorts(%+v) err = %v, want %s", o, err, codeInvalidSetting) + } + return + } + if base == 0 { + base = DefaultBasePort + } + if transfers == 0 { + transfers = DefaultTransfers + } + if len(ports) != transfers+1 { + t.Fatalf("relayPorts(%+v) gave %d ports, want %d", o, len(ports), transfers+1) + } + for i, p := range ports { + n, err := strconv.Atoi(p) + if err != nil || n != base+i || n < 1 || n > 65535 { + t.Fatalf("relayPorts(%+v)[%d] = %q, want %d within 1-65535", o, i, p, base+i) + } + } + }) +} + +var throttleSpeed = regexp.MustCompile(`^[0-9]+[kKmMgG]?$`) + +func FuzzValidateThrottle(f *testing.F) { + for _, s := range []string{"", "500k", " 2M ", "1g", "12", "5", "0k", "k", "kk", "-5k", "+5k", "5kb", "1.5m"} { + f.Add(s) + } + f.Fuzz(func(t *testing.T, value string) { + err := validateThrottle(value) + if err != nil { + if codeOf(err) != codeInvalidSetting { + t.Fatalf("validateThrottle(%q) err = %v, want %s", value, err, codeInvalidSetting) + } + return + } + v := strings.TrimSpace(value) + if v == "" { + return + } + // croc panics on anything but digits plus one optional unit letter, on + // zero, and ignores a one-character value. + if len(v) < 2 || !throttleSpeed.MatchString(v) || strings.Trim(v, "0kKmMgG") == "" { + t.Fatalf("validateThrottle(%q) accepted a value croc cannot parse", value) + } + }) +} + +func FuzzProxyURL(f *testing.F) { + for _, s := range []string{ + "", "127.0.0.1:1080", "proxy.example:3128", "socks5://user:secret@proxy.example:1080", + "socks5h://proxy.example:1080", "http://user:secret@proxy.example:3128", " https://proxy.example:443 ", + "proxy.example", "socks5://:1080", "user:secret@proxy.example", "proxy.example:0", "proxy.example:70000", + } { + f.Add(s) + } + f.Fuzz(func(t *testing.T, value string) { + for _, c := range []struct { + scheme string + allowed []string + }{ + {"socks5", []string{"socks5", "socks5h"}}, + {"http", []string{"http", "https"}}, + } { + u, err := proxyURL(value, c.scheme, c.allowed...) + if err != nil { + if u != nil || codeOf(err) != codeInvalidSetting { + t.Fatalf("proxyURL(%q, %s) = %v, %v; want nil and %s", value, c.scheme, u, err, codeInvalidSetting) + } + continue + } + if u == nil { + if strings.TrimSpace(value) != "" { + t.Fatalf("proxyURL(%q, %s) = nil, nil for a non-empty value", value, c.scheme) + } + continue + } + port, perr := strconv.Atoi(u.Port()) + if !slices.Contains(c.allowed, u.Scheme) || u.Hostname() == "" || perr != nil || port < 1 || port > 65535 { + t.Fatalf("proxyURL(%q, %s) accepted %q: scheme, host or port invalid", value, c.scheme, u) + } + } + }) +} diff --git a/crocmobile/osv-scanner.toml b/crocmobile/osv-scanner.toml new file mode 100644 index 00000000..703c91c9 --- /dev/null +++ b/crocmobile/osv-scanner.toml @@ -0,0 +1,5 @@ +# Read by OpenSSF Scorecard's Vulnerabilities check. Keep in step with docs/known-issues.md. + +[[IgnoredVulns]] +id = "GO-2026-5932" +reason = "golang.org/x/crypto/openpgp is never imported: croc only uses x/crypto/argon2, and govulncheck reports the vulnerability as not called. No fixed x/crypto version exists." diff --git a/crocmobile/session_test.go b/crocmobile/session_test.go index 2d179f34..6657eff2 100644 --- a/crocmobile/session_test.go +++ b/crocmobile/session_test.go @@ -845,3 +845,29 @@ func TestPollReportsFileListOfTransferEndedWithinATick(t *testing.T) { t.Fatalf("OnFileList called %d times, want 1", len(d.lists)) } } + +func FuzzSplitPaths(f *testing.F) { + for _, s := range []string{"", "/d/report.pdf \n\n /d/x\n", "a\nb\nc", "\n\n", " a \n b"} { + f.Add(s) + } + f.Fuzz(func(t *testing.T, joined string) { + got := splitPaths(joined) + var want []string + for _, line := range strings.Split(joined, "\n") { + if line != "" { + want = append(want, line) + } + } + if !reflect.DeepEqual(got, want) { + t.Fatalf("splitPaths(%q) = %q, want every non-empty line verbatim: %q", joined, got, want) + } + for _, p := range got { + if p == "" || strings.Contains(p, "\n") { + t.Fatalf("splitPaths(%q) returned entry %q", joined, p) + } + } + if again := splitPaths(strings.Join(got, "\n")); !reflect.DeepEqual(again, got) { + t.Fatalf("splitPaths does not round-trip: %q then %q", got, again) + } + }) +} diff --git a/docs/known-issues.md b/docs/known-issues.md index 7530b4e7..36bd58aa 100644 --- a/docs/known-issues.md +++ b/docs/known-issues.md @@ -72,7 +72,9 @@ None. - **The trust badge names the configured route, not the path that carried the data.** `TrustBadge` reads the relay setting, so a transfer that croc moved over the LAN, or over Tailcat under Connection = Automatic, still shows "Via the public croc relay" (or the custom relay). croc does not report the winning path to the engine. Accepted: the badge's claim that the server sees only encrypted data holds on every path. - **Receive list identity is by file name.** Duplicate names within one transfer are a croc-level concern, not a UI one. - **Gesture-level flows are not machine-verified.** Drag-and-drop, camera QR, and taps have no XCUITest coverage, by decision: unit tests cover non-UI logic (Go `go test -race`, Swift Testing in `CrocKitTests` and `CrocAppTests`, all in CI), transfers are covered by the harnesses, and UI flows are verified by hand. The rule is in `CONTRIBUTING.md` (Verification, Tests). -- **`braces` <= 3.0.3 (GHSA-vfj7-8cjw-p6xm) has no patched release.** Stack exhaustion on deeply nested brace patterns; it is reached only at build time through `chokidar` and `fast-glob` over our own docs paths, never from user input, and no override can lift it. `pnpm audit` lists it, and any Dependabot alert for it is dismissed with this reason. Remove this item when `braces` publishes a fix. +- **`braces` <= 3.0.3 (GHSA-vfj7-8cjw-p6xm) has no patched release.** Stack exhaustion on deeply nested brace patterns; it is reached only at build time through `chokidar` and `fast-glob` over our own docs paths, never from user input, and no override can lift it. `pnpm audit` lists it, and any Dependabot alert for it is dismissed with this reason. `web/docs/osv-scanner.toml` ignores it so OpenSSF Scorecard's Vulnerabilities check does not count it. Remove this item, and that entry, when `braces` publishes a fix. +- **`golang.org/x/crypto` carries GO-2026-5932 (the unmaintained `openpgp` package) with no fixed version.** The advisory covers the whole module, but nothing imports `openpgp`: croc uses only `x/crypto/argon2`, and `go tool govulncheck ./...` in `crocmobile/` reports it as required but not called. `crocmobile/osv-scanner.toml` ignores it for Scorecard. Remove both if x/crypto drops `openpgp` or a dependency starts importing it. +- **Four OpenSSF Scorecard alerts are dismissed as "won't fix".** Scorecard opens a code-scanning alert for every check below 10, and these cannot reach 10 with one maintainer: Branch-Protection (#1; 10 needs two approving reviewers with admins included, so nothing could merge; `main` keeps PRs, the `ci pass` check, linear history, and no force push or deletion), Code-Review (#5; needs approvals from someone other than the author), Maintained (#4; fires only while the repo is under 90 days old, created 2026-07-21), and CII-Best-Practices (#6; only the gold badge scores 10, and gold needs several developers). Dismissed 2026-10-17. Revisit Branch-Protection and Code-Review if a second maintainer joins. - **`xfer()` has no watchdog.** If croc ever blocks past context cancel and prompt-pipe close, `activeMu` is held for the process's lifetime and every later transfer returns "another transfer is active". Accepted: forcing the lock open while croc is still running would let two sessions mutate the same process-global state (`os.Chdir`, stdin/stdout swaps). - **`poll()` reads `croc.Client` scalar and slice fields with no synchronization** (croc's own goroutines write them unguarded). Degrades to inconsistent progress numbers; bounds-checked (`session.go`'s `progressSnapshot`) so it cannot panic. Accepted: the writes are croc's, so a fix needs upstream locking, and the worst case is a progress number that is briefly wrong. - **The harnesses fail at random against the public relays when they rate-limit.** Accepted: the limit is relay policy, and CI does not depend on it. A croc v11 relay admits 30 connections a minute from one address (`DEFAULT_SOURCE_JOIN_LIMIT`, `DEFAULT_JOIN_LIMIT_WINDOW`), and a multiplexed transfer opens 9 per peer (1 control, 8 data), so with both peers on one machine two transfers on the same pool relay inside a minute can be refused with `relay admission rate limited`. `verify-interop.sh`, `verify-app-mac.sh` and `verify-share-sim.sh` then fail at whichever scenario hits it; on 2026-10-12 back-to-back `verify-interop.sh` runs failed at scenarios 6, 7 and 10 (then the pool scenario, now 11) this way, and the pre-v11 runs saw the same thing from `croc.schollz.com`. `verify-interop.sh` scenario 11 waits 60 s and turns multiplexing off for that reason. CI avoids this: its `interop (local relay)` job runs `verify-interop.sh` with `RELAY` set to a local `croc relay` with raised admission limits (`docs/BUILDING.md`, section 5). The other harnesses have no local-relay mode yet, so use a local relay for a deterministic result, leave a minute between public-relay runs, and treat a failure as a regression only after finding the cause in its log. diff --git a/web/docs/osv-scanner.toml b/web/docs/osv-scanner.toml new file mode 100644 index 00000000..6640c5a2 --- /dev/null +++ b/web/docs/osv-scanner.toml @@ -0,0 +1,5 @@ +# Read by OpenSSF Scorecard's Vulnerabilities check. Keep in step with docs/known-issues.md. + +[[IgnoredVulns]] +id = "GHSA-vfj7-8cjw-p6xm" +reason = "braces <= 3.0.3 has no patched release. Reached only at docs build time through chokidar, micromatch and fast-glob over our own paths, never from user input."