sicd: v2 wire preserves argv boundaries (content = argv netstrings + terminator + payload)
The v2 command field was a single string the daemon SPACE-SPLIT into argv — which silently
broke sic's founding guarantee (`ssh host touch 'a b'` makes TWO files; `sic host touch 'a b'`
must make ONE). The composition of the v2 client surfaced this: every argument with a space
would be re-split.
Fix (markus chose it): v2 content is now a run of djb netstrings (the argv), terminated by the
empty netstring 0:,, then the payload — mirroring the boundary-safe v1 exec encoding. Each
argument is length-framed, so spaces, NUL, 0xFF all ride untouched; readNetstringStream already
bounds each element + its length token, so a hostile content cannot OOM here. runV2 uses the new
parseV2Content; the test frame() helper space-splits test-side only (the wire never does).
Proof: TestV2ArgvBoundaryPreserved — argv {script, "a b"} makes the script see 1 arg "a b", not
2. All existing v2 + v1 dual-read tests stay green; go vet clean.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
+34
-11
@@ -91,6 +91,29 @@ func readNetstringStream(r *bufio.Reader) ([]byte, bool) {
|
||||
return content, true
|
||||
}
|
||||
|
||||
// parseV2Content splits a v2 netstring's content into argv (a run of netstrings terminated by
|
||||
// the empty netstring 0:,) and the trailing payload. Length-framed argv preserves argument
|
||||
// boundaries and lets any byte (space, NUL, 0xFF) appear in an argument; readNetstringStream
|
||||
// bounds each element and its length token, so a hostile content cannot OOM here.
|
||||
func parseV2Content(content []byte) (argv [][]byte, payload []byte, ok bool) {
|
||||
r := bufio.NewReader(bytes.NewReader(content))
|
||||
for {
|
||||
ns, good := readNetstringStream(r)
|
||||
if !good {
|
||||
return nil, nil, false
|
||||
}
|
||||
if len(ns) == 0 {
|
||||
break // the empty netstring 0:, terminates argv
|
||||
}
|
||||
argv = append(argv, ns) // readNetstringStream returns a fresh slice; no aliasing
|
||||
}
|
||||
rest, err := io.ReadAll(r)
|
||||
if err != nil {
|
||||
return nil, nil, false
|
||||
}
|
||||
return argv, rest, true
|
||||
}
|
||||
|
||||
func waitForChild(cmd *exec.Cmd) int {
|
||||
if err := cmd.Wait(); err != nil {
|
||||
if exitErr, ok := err.(*exec.ExitError); ok {
|
||||
@@ -151,23 +174,23 @@ func runV2() {
|
||||
os.Exit(1)
|
||||
}
|
||||
|
||||
nulIdx := bytes.IndexByte(content, 0x00)
|
||||
if nulIdx < 0 {
|
||||
fmt.Fprintf(os.Stderr, "sicd: content missing NUL separator\n")
|
||||
// v2 content = argv netstrings + the empty-netstring terminator 0:, + payload. Length-framed
|
||||
// argv PRESERVES argument boundaries (sic's founding guarantee: `touch 'a b'` is ONE file)
|
||||
// and lets ANY byte appear in an argument — unlike the old space-split. Everything after the
|
||||
// terminator is the payload (a nested frame for the next hop, or empty).
|
||||
argv, payload, ok := parseV2Content(content)
|
||||
if !ok {
|
||||
fmt.Fprintf(os.Stderr, "sicd: invalid v2 content (argv framing)\n")
|
||||
os.Exit(1)
|
||||
}
|
||||
if len(argv) == 0 {
|
||||
fmt.Fprintf(os.Stderr, "sicd: empty command\n")
|
||||
os.Exit(1)
|
||||
}
|
||||
command := content[:nulIdx]
|
||||
payload := content[nulIdx+1:]
|
||||
|
||||
argv := bytes.Split(command, []byte(" "))
|
||||
argvStr := make([]string, len(argv))
|
||||
for i, a := range argv {
|
||||
argvStr[i] = string(a)
|
||||
}
|
||||
if len(argvStr) == 0 {
|
||||
fmt.Fprintf(os.Stderr, "sicd: empty command\n")
|
||||
os.Exit(1)
|
||||
}
|
||||
|
||||
cmd := exec.Command(argvStr[0], argvStr[1:]...)
|
||||
stdinPipe, err := cmd.StdinPipe()
|
||||
|
||||
+35
-1
@@ -75,7 +75,15 @@ func be32(n uint32) []byte {
|
||||
|
||||
// frame builds one wire frame: magic, 4-byte BE length, netstring(command NUL payload).
|
||||
func frame(command, payload []byte) []byte {
|
||||
ns := netstring(concat(command, []byte{0x00}, payload))
|
||||
// v2 content = argv netstrings + the empty-netstring terminator 0:, + payload. The command is
|
||||
// split on spaces HERE (test-side convenience); the wire itself no longer space-splits — argv
|
||||
// arrives length-framed so `touch 'a b'` keeps its boundary.
|
||||
var content []byte
|
||||
for _, a := range bytes.Split(command, []byte(" ")) {
|
||||
content = concat(content, netstring(a))
|
||||
}
|
||||
content = concat(content, []byte("0:,"), payload)
|
||||
ns := netstring(content)
|
||||
return concat([]byte{0x00}, be32(uint32(len(ns))), ns)
|
||||
}
|
||||
|
||||
@@ -590,3 +598,29 @@ func TestPumpStdinCleanEofReturnsChildStatus(t *testing.T) {
|
||||
t.Fatalf("a clean EOF is a legitimate end; must return the child's status 7, got %d", code)
|
||||
}
|
||||
}
|
||||
|
||||
// v2FrameArgv builds a v2 frame from EXPLICIT argv (no space-split) — the whole point of the
|
||||
// argv-netstring wire: each element is length-framed and rides untouched.
|
||||
func v2FrameArgv(argv [][]byte, payload []byte) []byte {
|
||||
var content []byte
|
||||
for _, a := range argv {
|
||||
content = concat(content, netstring(a))
|
||||
}
|
||||
content = concat(content, []byte("0:,"), payload)
|
||||
ns := netstring(content)
|
||||
return concat([]byte{0x00}, be32(uint32(len(ns))), ns)
|
||||
}
|
||||
|
||||
func TestV2ArgvBoundaryPreserved(t *testing.T) {
|
||||
// sic's founding guarantee, now on the v2 wire: an argument containing a space is ONE argv
|
||||
// element, never re-split. Before the argv-netstring wire this failed — the space-split gave
|
||||
// the script two args and printed "2|a". This is the regression that pins the fix.
|
||||
script := shScript(t, `printf '%s|%s\n' "$#" "$1"`)
|
||||
r := runSicd(t, v2FrameArgv([][]byte{[]byte(script), []byte("a b")}, nil))
|
||||
if r.code != 0 {
|
||||
t.Fatalf("expected exit 0, got %d, stderr=%q", r.code, r.stderr)
|
||||
}
|
||||
if got, want := string(r.stdout), "1|a b\n"; got != want {
|
||||
t.Fatalf("v2 argv boundary LOST: script saw %q, want %q", got, want)
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user