Go 1.26, SAST and hostile-input hardening, go-modbus v1.11.1 (v3.5.0)

.github/workflows/ci.yml

@@ -19,7 +19,7 @@ jobs:
       coverage-min: "85"
       fuzz-time: 10s
       codecov-name: modbusctl
-      go-versions: '["1.26"]'
+      go-versions: '["1.26", "1.27"]'
       # Test the cmd packages as well (the shared default skips /cmd) and attribute
       # coverage across the whole module, matching `make coverage`.
       test-exclude-regex: "/example(s)?($|/)"

README.md

@@ -928,7 +928,7 @@ modbusctl server static --input site.mbcap --target 2 --unit 1 --at 10m
 
 #### Replay recorded data (looped)
 
-The served values follow the recording: at its original speed (`--speed 1`), faster (`--speed 60`: a minute per second) or slower (`--speed 0.1`). `--loops 0` (default) repeats forever; `--loops N` plays N times and then keeps the last values. `--from` starts part-way into the capture.
+The served values follow the recording: at its original speed (`--speed 1`), faster (`--speed 60`: a minute per second, up to `31536000`: a year per second) or slower (`--speed 0.1`). `--loops 0` (default) repeats forever; `--loops N` plays N times and then keeps the last values. `--from` starts part-way into the capture.
 
 ```console
 modbusctl server replay --input session.mbcap --port 1502

RELEASE.md

@@ -1,5 +1,35 @@
 # modbusctl Releases
 
+## v3.5.0
+
+**Date:** 2026-11-05
+**Previous release:** v3.4.3
+
+### Security
+
+- Text and table output, and the `error:` line, escape control characters, Unicode bidi controls and bytes that are not UTF-8 in strings from devices and files: FC43 identification, ASCII and UTF-16 codec values in `client decode` and `client poll`, profile names in `capture decode`. A device could inject terminal escape sequences or forge lines. JSON output is unchanged.
+- modbusctl no longer writes through a symbolic link at an output path (exit 10, `output_file_create_failed`). New captures, exports and `discover -o` files are created with mode 0600 instead of 0644.
+
+### Fixed
+
+- `--output` naming the command's own input (the input or profile of `capture decode`, `extract`, `strings` and `frequencies`, or the blocks file of `client record`) exits 3 (`invalid_flag_combination`) instead of truncating the input before reading it.
+- `client scan --algo deep` no longer reads addresses outside `--start`/`--end` when the range ends at 65535 (the address counter wrapped to 0).
+- `server replay --speed` refuses NaN, infinities and values above 31536000 (exit 3); the replay clock stays inside the capture window at extreme speeds. Replay no longer answers reads beyond one response (more than 125 registers or 2000 coils) with a wrapped byte count.
+- `discover` refuses an oversized subnet (`0.0.0.0/0`, an IPv6 /64) at once instead of enumerating every host first; the error message text changed.
+
+### Changed
+
+- Blocks files and device profiles larger than 16 MiB are refused (exit 3).
+- The healthcheck binary has a 10 s timeout per check, and an `-http` value that is not an http(s) URL with a host exits 2.
+- CI runs Go 1.26 and 1.27; `go fix` modernised the code. Built on go-modbus v1.11.1: the server closes a connection whose response cannot be written in time; RTU and ASCII links drain a late answer instead of returning it for the next request; RTU variable-length reads honour the deadline; UDP drops bytes left over from a previous datagram.
+- Integer conversions are range-checked and the pty helpers use golang.org/x/sys/unix instead of unsafe: a static scan reports no finding.
+
+### Tests
+
+- Hostile client, device-string, file and flag tests, and new fuzz targets for replay reads and clock, terminal escaping, capture analyses, device-profile decoding and replay speed.
+
+---
+
 ## v3.4.3
 
 **Date:** 2026-11-03

cmd/capture/decode.go

@@ -41,9 +41,9 @@ func init() {
 		if err := validate.CheckDeviceProfileDecodeConfig(cfg); err != nil {
 			return errs.WrapValidation(err)
 		}
-		w, cleanup, err := cli.OpenStdoutOrFile(cfg.OutputFile)
+		w, cleanup, err := cli.OpenStdoutOrFile(cfg.OutputFile, cfg.InputFile, cfg.DeviceProfile)
 		if err != nil {
-			return errs.Output(errs.CodeOutputFileCreateFailed, err)
+			return cli.OutputOpenError(err)
 		}
 		defer cleanup()
 

cmd/capture/extract.go

@@ -38,9 +38,9 @@ func init() {
 		if err := validate.CheckExtractConfig(cfg); err != nil {
 			return errs.WrapValidation(err)
 		}
-		w, cleanup, err := cli.OpenStdoutOrFile(cfg.OutputFile)
+		w, cleanup, err := cli.OpenStdoutOrFile(cfg.OutputFile, cfg.InputFile)
 		if err != nil {
-			return errs.Output(errs.CodeOutputFileCreateFailed, err)
+			return cli.OutputOpenError(err)
 		}
 		defer cleanup()
 

cmd/capture/frequencies.go

@@ -41,9 +41,9 @@ func init() {
 		if err := validate.CheckStringsConfig(cfg); err != nil {
 			return errs.WrapValidation(err)
 		}
-		w, cleanup, err := cli.OpenStdoutOrFile(cfg.OutputFile)
+		w, cleanup, err := cli.OpenStdoutOrFile(cfg.OutputFile, cfg.InputFile)
 		if err != nil {
-			return errs.Output(errs.CodeOutputFileCreateFailed, err)
+			return cli.OutputOpenError(err)
 		}
 		defer cleanup()
 

cmd/capture/hostile_files_test.go

@@ -0,0 +1,105 @@
+// SPDX-License-Identifier: MIT
+
+package capture
+
+import (
+	"os"
+	"path/filepath"
+	"runtime"
+	"strings"
+	"testing"
+
+	"github.com/otfabric/modbusctl/internal/capfile/capfiletest"
+	"github.com/otfabric/modbusctl/internal/errs"
+	"github.com/otfabric/modbusctl/internal/fsutil"
+	"github.com/otfabric/modbusctl/internal/types"
+)
+
+// TestOutputNamingTheInput_isRefusedBeforeTruncating: --output naming the capture
+// (or the profile) truncated it before it was read, losing the recording.
+func TestOutputNamingTheInput_isRefusedBeforeTruncating(t *testing.T) {
+	in := recordCapture(t)
+	before := readFile(t, in)
+	profile := filepath.Join(t.TempDir(), "profile.json")
+	writeFile(t, profile, `{"protocolData":{"registers":[{"controlledPropertyId":"f","valueScaleFactor":1,"start":104,"size":1,"format":">H"}]}}`)
+	profileBefore := readFile(t, profile)
+	// Another spelling of the input path names the same file.
+	alias := filepath.Join(filepath.Dir(in), ".", filepath.Base(in))
+	for _, args := range [][]string{
+		{"capture", "extract", "--input", in, "--output", in},
+		{"capture", "strings", "--input", in, "--output", alias},
+		{"capture", "frequencies", "--input", in, "--output", in},
+		{"capture", "decode", "--input", in, "--profile", profile, "--output", in},
+		{"capture", "decode", "--input", in, "--profile", profile, "--output", profile},
+	} {
+		r := run(t, args...)
+		wantExit(t, r, errs.ExitInvalidInput)
+		if e, _ := errs.As(r.err); e.Code != errs.CodeInvalidFlagCombination {
+			t.Fatalf("%v: code %q", args, e.Code)
+		}
+	}
+	if readFile(t, in) != before || readFile(t, profile) != profileBefore {
+		t.Fatal("an input was truncated")
+	}
+}
+
+// TestOutputs_ownerOnlyAndNeverThroughSymlinks: outputs were created 0644 and written
+// through a symbolic link planted at the output path.
+func TestOutputs_ownerOnlyAndNeverThroughSymlinks(t *testing.T) {
+	in := recordCapture(t)
+	dir := t.TempDir()
+	out := filepath.Join(dir, "blocks.json")
+	wantExit(t, run(t, "capture", "extract", "--input", in, "--output", out), errs.ExitOK)
+	if st, err := os.Stat(out); err != nil || (runtime.GOOS != "windows" && st.Mode().Perm() != fsutil.OutputMode) {
+		t.Fatalf("output mode: %v %v", st.Mode(), err)
+	}
+
+	victim := filepath.Join(dir, "victim")
+	writeFile(t, victim, "keep")
+	link := filepath.Join(dir, "link.json")
+	if err := os.Symlink(victim, link); err != nil {
+		t.Skipf("no symlinks: %v", err)
+	}
+	r := run(t, "capture", "extract", "--input", in, "--output", link)
+	wantExit(t, r, errs.ExitInternal)
+	if readFile(t, victim) != "keep" {
+		t.Fatal("written through the symbolic link")
+	}
+}
+
+// TestDecodeProfile_hostileProfiles: a profile is a file from elsewhere.
+func TestDecodeProfile_hostileProfiles(t *testing.T) {
+	dir := t.TempDir()
+	// A capture holding registers 65534, 65535 and 0.
+	in := capfiletest.WriteRecords(t, filepath.Join(dir, "top.mbcap"), types.CaptureHeader{IP: "10.0.0.1", Port: 502, Unit: 1, Function: 3},
+		types.CaptureRecord{Iteration: 1, StartAddress: 65534, RegisterCount: 2, Data: []byte{0x12, 0x34, 0x56, 0x78}},
+		types.CaptureRecord{Iteration: 1, StartAddress: 0, RegisterCount: 1, Data: []byte{0xAA, 0xBB}})
+	out := filepath.Join(dir, "out.txt")
+
+	// A register running past 65535 is refused by the profile check (exit 3);
+	// analysis.ExportDeviceProfileDecode guards against it as well.
+	profile := filepath.Join(dir, "wrap.json")
+	writeFile(t, profile, `{"protocolData":{"registers":[
+		{"controlledPropertyId":"top","valueScaleFactor":1,"start":65535,"size":2,"format":">I"}]}}`)
+	wantExit(t, run(t, "capture", "decode", "--input", in, "--profile", profile, "--output", out), errs.ExitInvalidInput)
+
+	// Names go to a terminal: control characters are escaped.
+	profile = filepath.Join(dir, "names.json")
+	writeFile(t, profile, `{"protocolData":{"registers":[{"controlledPropertyId":"x\u001b[2J","valueScaleFactor":1,"start":65534,"size":1,"format":">H"}]}}`)
+	wantExit(t, run(t, "capture", "decode", "--input", in, "--profile", profile, "--output", out), errs.ExitOK)
+	if got := readFile(t, out); strings.ContainsRune(got, 0x1b) || !strings.Contains(got, `x\x1b[2J = 4660.000000`) {
+		t.Fatalf("output %q", got)
+	}
+
+	// A profile larger than any real one is refused without reading it whole.
+	big := filepath.Join(dir, "big.json")
+	f, err := os.Create(big)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if err := f.Truncate(fsutil.MaxJSONInput + 1); err != nil {
+		t.Fatal(err)
+	}
+	_ = f.Close()
+	wantExit(t, run(t, "capture", "decode", "--input", in, "--profile", big, "--output", out), errs.ExitInvalidInput)
+}

cmd/capture/strings.go

@@ -41,9 +41,9 @@ func init() {
 		if err := validate.CheckStringsConfig(cfg); err != nil {
 			return errs.WrapValidation(err)
 		}
-		w, cleanup, err := cli.OpenStdoutOrFile(cfg.OutputFile)
+		w, cleanup, err := cli.OpenStdoutOrFile(cfg.OutputFile, cfg.InputFile)
 		if err != nil {
-			return errs.Output(errs.CodeOutputFileCreateFailed, err)
+			return cli.OutputOpenError(err)
 		}
 		defer cleanup()
 

cmd/client/client_test.go

@@ -465,7 +465,7 @@ func TestWrite_transports(t *testing.T) {
 func pollLines(t *testing.T, stdout string) []types.PollSample {
 	t.Helper()
 	var samples []types.PollSample
-	for _, line := range strings.Split(strings.TrimSpace(stdout), "\n") {
+	for line := range strings.SplitSeq(strings.TrimSpace(stdout), "\n") {
 		var s types.PollSample
 		decodeJSON(t, line, &s)
 		samples = append(samples, s)

cmd/client/hostile_output_test.go

@@ -0,0 +1,75 @@
+// SPDX-License-Identifier: MIT
+
+package client
+
+import (
+	"strings"
+	"testing"
+
+	"github.com/otfabric/modbusctl/internal/errs"
+	"github.com/otfabric/modbusctl/internal/mbtest"
+	"github.com/otfabric/modbusctl/internal/types"
+)
+
+// hostileText is what a hostile device puts in its strings: a screen clear, a
+// window-title change, a carriage return that overwrites the line, a fake extra
+// line, a tab that breaks table columns, a bidi override and a C1 CSI.
+const hostileText = "Acme\x1b[2J\x1b]0;owned\x07\rFAKE\nObject 9: forged\tcol\u202eevil\u009b31m"
+
+// requireTerminalSafe fails when out carries a control character other than a
+// newline (and a tab in text output), or a bidi override.
+func requireTerminalSafe(t *testing.T, what, out string, tabs bool) {
+	t.Helper()
+	for i, r := range out {
+		ok := r == '\n' || (tabs && r == '\t') || (r >= 0x20 && r != 0x7f && (r < 0x80 || r > 0x9f))
+		if !ok || (r >= 0x202a && r <= 0x202e) {
+			t.Fatalf("%s: control character %U at byte %d reaches the terminal:\n%q", what, r, i, out)
+		}
+	}
+}
+
+// TestHostileDevice_identifyStringsAreEscaped: FC43 objects are device strings.
+// Text and table output escape their control characters; JSON keeps the value.
+func TestHostileDevice_identifyStringsAreEscaped(t *testing.T) {
+	d, _ := newDevice(t)
+	d.Identification = map[uint8]string{0: hostileText, 1: "P-1000", 2: "1.2.3"}
+	d.Units = []uint8{1}
+	url := "tcp://" + d.ServeTCP(t)
+
+	r := run(t, "client", "identify", "--url", url)
+	wantExit(t, r, errs.ExitOK)
+	requireTerminalSafe(t, "identify text", r.stdout, true)
+	wantContains(t, r.stdout, `Object 0: Acme\x1b[2J\x1b]0;owned\x07\rFAKE\nObject 9: forged\tcol\u202eevil\x9b31m [VendorName]`)
+	if strings.Count(r.stdout, "\n - Object ") != 3 {
+		t.Fatalf("the device string forged an output line:\n%s", r.stdout)
+	}
+
+	r = run(t, "client", "identify", "--url", url, "--format", "table")
+	wantExit(t, r, errs.ExitOK)
+	requireTerminalSafe(t, "identify table", r.stdout, false)
+
+	r = run(t, "client", "identify", "--url", url, "--format", "json")
+	wantExit(t, r, errs.ExitOK)
+	var res types.IdentifyResult
+	decodeJSON(t, r.stdout, &res)
+	if got := res.Units[0].Objects[0].Value; got != hostileText {
+		t.Fatalf("JSON value %q, want the device string unchanged %q", got, hostileText)
+	}
+}
+
+// TestHostileDevice_decodedStringsAreEscaped: an ASCII codec turns register bytes
+// into a string; text and table output escape its control characters.
+func TestHostileDevice_decodedStringsAreEscaped(t *testing.T) {
+	// "\x1b[2J" + "\r\nX" in four registers.
+	d := mbtest.NewDevice().SetHolding(0, 0x1b5b, 0x324a, 0x0d0a, 0x5807)
+	url := "tcp://" + d.ServeTCP(t)
+	for _, f := range []string{"text", "table"} {
+		r := run(t, "client", "decode", "--url", url, "--function", "3", "--start", "0", "--codec", "ascii_fixed/registers:4", "--format", f)
+		wantExit(t, r, errs.ExitOK)
+		requireTerminalSafe(t, "decode "+f, r.stdout, f == "text")
+		wantContains(t, r.stdout, `\x1b[2J\r\nX\x07`)
+	}
+	r := run(t, "client", "poll", "--url", url, "--function", "3", "--start", "0", "--codec", "ascii_fixed/registers:4", "--polls", "1", "--interval", "1")
+	wantExit(t, r, errs.ExitOK)
+	requireTerminalSafe(t, "poll text", r.stdout, true)
+}

cmd/server/hostile_flags_test.go

@@ -0,0 +1,22 @@
+// SPDX-License-Identifier: MIT
+
+package server
+
+import (
+	"testing"
+
+	"github.com/otfabric/modbusctl/internal/errs"
+)
+
+// TestReplay_nonFiniteSpeedIsRefused: "NaN" and "Inf" parse as floats and passed
+// the positive check (NaN <= 0 is false), starting a replay whose time was
+// undefined. They exit 3 now, as does a speed above a year per second.
+func TestReplay_nonFiniteSpeedIsRefused(t *testing.T) {
+	in, _ := recordHolding(t)
+	port := freePort(t)
+	for _, speed := range []string{"NaN", "Inf", "+Inf", "infinity", "1e300", "31536001"} {
+		r := run(t, "server", "replay", "--input", in, "--port", port, "--speed", speed)
+		wantExit(t, r, errs.ExitInvalidInput)
+		wantContains(t, r.err.Error(), "invalid --speed")
+	}
+}

cmd/server/tls_test.go

@@ -57,10 +57,12 @@ func recordTLS(t *testing.T, p *mbtest.PKI) (string, uint16) {
 }
 
 // readTLS reads holding registers 100-102 from the server with the modbusctl TLS client.
-func readTLS(s *server, tlsCfg config.TLSConfig) (string, error) {
+// The capture of the read goes to the test's temporary directory, not the package directory.
+func readTLS(t *testing.T, s *server, tlsCfg config.TLSConfig) (string, error) {
+	t.Helper()
 	res, err := modbus.CollectRead(context.Background(), config.ReadConfig{
 		DeviceConfig: config.DeviceConfig{URL: "tcp+tls://127.0.0.1:" + s.port, Unit: 1, TLSConfig: tlsCfg},
-		Timeout:      2000, Function: 3, StartAddress: 100, RegisterCount: 3,
+		Timeout:      2000, Function: 3, StartAddress: 100, RegisterCount: 3, OutputFile: filepath.Join(t.TempDir(), "read.mbcap"),
 	}, nil)
 	if err != nil {
 		return "", err
@@ -96,7 +98,7 @@ func TestTLS_staticAndReplay(t *testing.T) {
 	last := hexWords(iterations, 0xBEEF, 0x1234)
 
 	s := startServer(t, append([]string{"static", "--input", in}, serverTLSFlags(pki)...)...)
-	got, err := readTLS(s, clientTLS(pki))
+	got, err := readTLS(t, s, clientTLS(pki))
 	if err != nil || got != last {
 		t.Fatalf("static over tcp+tls: %q, %v; want %q", got, err, last)
 	}
@@ -108,7 +110,7 @@ func TestTLS_staticAndReplay(t *testing.T) {
 	s = startServer(t, append([]string{"replay", "--input", in, "--loops", "1", "--speed", "4"}, serverTLSFlags(pki)...)...)
 	deadline := time.Now().Add(5 * time.Second)
 	for {
-		got, err := readTLS(s, clientTLS(pki))
+		got, err := readTLS(t, s, clientTLS(pki))
 		if err != nil {
 			t.Fatalf("replay over tcp+tls: %v", err)
 		}
@@ -132,7 +134,7 @@ func TestTLS_untrustedClientRejected(t *testing.T) {
 	in, _ := recordTLS(t, pki)
 	for _, sub := range []string{"static", "replay"} {
 		s := startServer(t, append([]string{sub, "--input", in}, serverTLSFlags(pki)...)...)
-		if _, err := readTLS(s, config.TLSConfig{TLSCert: other.ClientCertFile, TLSKey: other.ClientKeyFile, TLSCA: pki.CAFile}); err == nil {
+		if _, err := readTLS(t, s, config.TLSConfig{TLSCert: other.ClientCertFile, TLSKey: other.ClientKeyFile, TLSCA: pki.CAFile}); err == nil {
 			t.Fatalf("server %s answered a client certificate of another CA", sub)
 		}
 		if _, err := readHolding(t, s.dial(t), 1, 100, 1); err == nil {

docs/exitcodes.md

@@ -32,6 +32,8 @@
 
 **Capture files:** a modbusctl 2.x capture file (`capture_legacy_format`), and a `--target`/`--function` selection that matches several or no targets and functions of a capture (`capture_selection_ambiguous`, `capture_selection_empty`), and a capture holding a sample of a vendor or unknown function code, outside the Modbus read functions modbusctl supports (`capture_unsupported_function`), exit `3`. A capture file that cannot be read, or is corrupt or truncated where a command needs it complete (`capture_load_failed`), exits `3`; `server static` and `server replay` serve a truncated capture as far as it goes. Failing to write a capture (`capture_write_failed`) exits `10`.
 
+**Files named on the command line:** an output that names an input file of the same command (`--output` naming the `--input` capture or the `--profile` of `capture decode`, `extract`, `strings` and `frequencies`, or the `--blocks-file` of `client record`, by any path or link) exits `3` (`invalid_flag_combination`) before anything is truncated. An output path that is a symbolic link is refused (`output_file_create_failed`, exit `10`): modbusctl does not write through links. A blocks file or device profile larger than 16 MiB exits `3`. Files modbusctl creates (captures, exports, discovery results) are owner-only (`0600`).
+
 **Interruption with a result:** an interrupted `fingerprint` writes its result and exits `7`, and `client poll` ends normally (see below). `server static` and `server replay` serve until they are stopped: Ctrl-C ends them normally with `0`.
 
 **Format variable:** `MODBUSCTL_FORMAT` is the name to use; `MODBUSCTL_OUTPUT_FORMAT` stays accepted. When both are set, `MODBUSCTL_FORMAT` wins.

docs/json-output.md

@@ -16,6 +16,8 @@ These conventions are a **stable contract** for scripting. Prefer **typed struct
 | Poll sample (stream) | `PollSample`, **one compact JSON object per line** (JSON Lines), written as each poll completes: `poll` (1-based), `timestamp` (RFC 3339, UTC, milliseconds), `unit_id`, `function`, `start_address`, `count`, `address_step` (address distance between consecutive `values`: the codec register width, or 1), `rtt_ms`, `raw_data_hex`, `values` (booleans for functions 1/2, numbers for 3/4, codec-typed values with `codec`), and `error` (`kind`, `code`, `message`) on a failed poll, in which case `values` is empty. The end-of-run summary goes to stderr as text |
 | Record final summary | `RecordSummaryResult`: `target`, `blocks_recorded`, `iterations`, `capture_output_path` (the MBCAP capture file written) (per-iteration progress stays on stderr) |
 
+**Strings from devices and files** (FC43 identification objects, codec-decoded strings, device profile names): JSON carries the exact value (encoding/json escapes control characters). Text and table output, and the `error:` line on stderr, write control characters, Unicode bidirectional controls and bytes that are not UTF-8 as Go escapes (`\x1b`, `\n`, `\u202e`, `\xff`), so a device cannot drive the terminal or forge output lines.
+
 ## Error object
 
 Used for a fatal error (`{"error": {…}}`) and for a row that failed.