fix(transcoders): return error on short onion/onion3 input instead of panicking

[SAY-5]

The onionBtS and onion3BtS transcoders index their input at fixed offsets without first checking len(b). The fixed-size Cast / validateComponent paths happen to validate the length, but the transcoders are exported (TranscoderOnion / TranscoderOnion3) so direct callers — or any future code path that reaches BytesToString before validation — see a slice bounds out of range panic on short input. Mirror the existing onionValidate/onion3Validate length checks at the top of each transcoder and return an error instead.

Closes #288

[kajaaz]

Great fix for onionBtS / onion3BtS from #288. While reviewing the same class of defect across the full transcoder suite, I noticed portBtS is the one remaining fixed-size transcoder whose BtS function has no internal length check, and TranscoderPort is registered with a nil validator:

var TranscoderPort = NewTranscoderFromFunctions(portStB, portBtS, nil)  // nil = no validator

func portBtS(b []byte) (string, error) {
    i := binary.BigEndian.Uint16(b)   // assumes len(b) >= 2, no guard
    return strconv.FormatUint(uint64(i), 10), nil
}

In isolation portBtS(b) with len(b) < 2 would panic in binary.BigEndian.Uint16. In practice this is not reachable through Cast / NewMultiaddrBytes: validateBytes and readComponent (both in codec.go) enforce the fixed 2-byte size for the port protocols (tcp, udp, dccp, sctp : all Size: 16) before any BtS call, so portBtS only ever sees a 2-byte slice on those paths. A short component is rejected earlier with invalid value for size N; e.g. Cast([]byte{0x06, 0x00}) panics with "multiaddr failed to parse: invalid value for size 1", not an out-of-range panic inside Uint16. I confirmed there is no path within the library that delivers a short slice to portBtS through the public API.

So this is not a reachable panic today, but it is a consistency and defense-in-depth gap. Every other fixed-size transcoder either guards internally (e.g. ipcidrBtS, ip6zoneBtS, httpPathBtS) or supplies a validator, and portBtS does neither. Aligning it with the rest seems worthwhile alongside the onion fixes in this PR.

Suggested change:

func portBtS(b []byte) (string, error) {
    if len(b) < 2 {
        return "", fmt.Errorf("port: byte slice too short: %d bytes, want 2", len(b))
    }
    i := binary.BigEndian.Uint16(b)
    return strconv.FormatUint(uint64(i), 10), nil
}

func portValidate(b []byte) error {
    if len(b) != 2 {
        return fmt.Errorf("port: invalid length: %d bytes, want 2", len(b))
    }
    return nil
}

var TranscoderPort = NewTranscoderFromFunctions(portStB, portBtS, portValidate)

Discovery : Zorya concolic executor

Found while auditing the full transcoders.go BtS suite with Zorya, a concolic execution engine for Go binaries. Zorya symbolized the length n of the byte slice fed to portBtS and used Z3 constraint solving to enumerate all OOB branches. The harness targeted portBtS directly (function mode, address 0x498380) with a safe 8-byte seed (--arg "0800000000000000" = little-endian int64 8); Zorya then negated path constraints to discover the panicking inputs.

zorya zorya_port_real \
  --mode function 0x498380 \
  --lang go --compiler gc \
  --thread-scheduling main-only \
  --arg "0800000000000000" \
  --negate-path-exploration

Three SAT states were produced in under 90 seconds:

Z3 witness	Opcode	Elapsed	Instruction / Panic addr	Behaviour
n = MinInt64 (0x8000000000000000)	INT_SUB	74 s	0x498395 / 0x498395	Signed subtraction wraps in runtime bounds check
n = 9 (past 8-byte source buffer)	CBRANCH	79 s	0x498398 / 0x4983b5	Slice OOB on harness source buffer
n = 0	CBRANCH	90 s	0x498318 / 0x498338	binary.BigEndian.Uint16([]byte{}) → index out of range [1] with length 0

The decisive witness is n = 0: instruction 0x498318 maps directly to the binary.BigEndian.Uint16(b) call inside portBtS, and panic address 0x498338 is the runtime bounds-check abort. The n = 0 and n = MinInt64 witnesses together confirm both the missing lower-bound guard and the absence of a negative-index check.

For full disclosure: the finding is the missing guard itself; reachability is bounded by validateBytes / readComponent in the public API, so this should be treated as hardening rather than a security fix.

[SAY-5]

Thanks for the thorough audit. Pushed 3aac990 mirroring the suggestion: portBtS gets a len < 2 guard returning an error rather than panicking, portValidate enforces len == 2, and TranscoderPort now registers it instead of nil. Test covers nil/1-byte BytesToString, wrong-length Validate, and the 2-byte happy path.