Fix symlink race condition (TOCTOU): use O_NOFOLLOW to open files

The rsync vulnerability CVE-2024-12747 also affected gokrazy/rsync
(before this commit): An attacker could replace a regular file
(= contents will be transferred) with a symlink pointing outside
of the specified directory.

To reproduce the issue, run TestReceiverSymlinkTraversal with the
following change to rsyncd/rsyncd.go applied to simulate an attack:

    --- i/rsyncd/rsyncd.go
    +++ w/rsyncd/rsyncd.go
    @@ -373,6 +375,16 @@ func (s *Server) HandleConn(module Module, rd io.Reader, crd *countingReader, cw

     	s.logger.Printf("file list sent")

    +	// HACK: swap out the passwd file with a symlink to /etc/passwd
    +	if err := os.Remove(filepath.Join(module.Path, "passwd")); err != nil {
    +		return err
    +	}
    +	if err := os.Symlink("../passwd", filepath.Join(module.Path, "passwd")); err != nil {
    +		return err
    +	}
    +	s.logger.Printf("HACK: swapped passwd file for symlink")
    +
     	// Sort the file list. The client sorts, so we need to sort, too (in the
     	// same way!), otherwise our indices do not match what the client will
     	// request.

Before this commit, the test would transfer the "secret" contents;
after this commit, the test fails with "no such file or directory".

internal/nofollow/nofollow_others.go

@@ -0,0 +1,7 @@
+//go:build !unix
+
+package nofollow
+
+// Maybe resolves to unix.O_NOFOLLOW on unix systems,
+// 0 on other platforms. TODO(go1.24): use os.Root.
+const Maybe = 0

internal/nofollow/nofollow_unix.go

@@ -0,0 +1,9 @@
+//go:build unix
+
+package nofollow
+
+import "golang.org/x/sys/unix"
+
+// Maybe resolves to unix.O_NOFOLLOW on unix systems,
+// 0 on other platforms. TODO(go1.24): use os.Root.
+const Maybe = unix.O_NOFOLLOW

internal/receivermaincmd/generator.go

@@ -10,6 +10,7 @@ import (
 
 	"github.com/gokrazy/rsync"
 	"github.com/gokrazy/rsync/internal/log"
+	"github.com/gokrazy/rsync/internal/nofollow"
 	"github.com/gokrazy/rsync/internal/rsyncchecksum"
 	"github.com/gokrazy/rsync/internal/rsynccommon"
 )
@@ -244,7 +245,7 @@ func (rt *recvTransfer) recvGenerator(idx int, f *file) error {
 
 	// TODO: if deltas are disabled, request the file in full
 
-	in, err := os.Open(local)
+	in, err := os.OpenFile(local, os.O_RDONLY|nofollow.Maybe, 0)
 	if err != nil {
 		log.Printf("failed to open %s, continuing: %v", local, err)
 		return requestFullFile()

internal/receivermaincmd/receiver.go

@@ -10,6 +10,7 @@ import (
 
 	"github.com/gokrazy/rsync"
 	"github.com/gokrazy/rsync/internal/log"
+	"github.com/gokrazy/rsync/internal/nofollow"
 	"github.com/mmcloughlin/md4"
 )
 
@@ -54,7 +55,7 @@ func (rt *recvTransfer) recvFile1(f *file) error {
 func (rt *recvTransfer) openLocalFile(f *file) (*os.File, error) {
 	local := filepath.Join(rt.dest, f.Name)
 
-	in, err := os.Open(local)
+	in, err := os.OpenFile(local, os.O_RDONLY|nofollow.Maybe, 0)
 	if err != nil {
 		return nil, err
 	}

receiver_test.go

@@ -339,3 +339,47 @@ func TestReceiverCommand(t *testing.T) {
 		}
 	}
 }
+
+// TestReceiverSymlinkTraversal passes by default but is useful to simulate
+// a symlink race TOCTOU attack by modifying rsyncd/rsyncd.go.
+func TestReceiverSymlinkTraversal(t *testing.T) {
+	tmp := t.TempDir()
+	if err := os.WriteFile(filepath.Join(tmp, "passwd"), []byte("secret"), 0644); err != nil {
+		t.Fatal(err)
+	}
+	source := filepath.Join(tmp, "source")
+	dest := filepath.Join(tmp, "dest")
+
+	if err := os.MkdirAll(source, 0755); err != nil {
+		t.Fatal(err)
+	}
+	hello := filepath.Join(source, "passwd")
+	if err := os.WriteFile(hello, []byte("benign"), 0644); err != nil {
+		t.Fatal(err)
+	}
+
+	// start a server to sync from
+	srv := rsynctest.New(t, rsynctest.InteropModule(source))
+
+	args := []string{
+		"gokr-rsync",
+		"-aH",
+		"rsync://localhost:" + srv.Port + "/interop/",
+		dest,
+	}
+	_, err := receivermaincmd.Main(args, os.Stdin, os.Stdout, os.Stdout)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	{
+		want := []byte("benign")
+		got, err := os.ReadFile(filepath.Join(dest, "passwd"))
+		if err != nil {
+			t.Fatal(err)
+		}
+		if diff := cmp.Diff(want, got); diff != "" {
+			t.Fatalf("unexpected file contents: diff (-want +got):\n%s", diff)
+		}
+	}
+}

rsyncd/match.go

@@ -8,14 +8,15 @@ import (
 	"os"
 
 	"github.com/gokrazy/rsync"
+	"github.com/gokrazy/rsync/internal/nofollow"
 	"github.com/gokrazy/rsync/internal/rsyncchecksum"
 	"github.com/mmcloughlin/md4"
 )
 
 // rsync/match.c:hash_search
 func (st *sendTransfer) hashSearch(targets []target, tagTable map[uint16]int, head rsync.SumHead, fileIndex int32, fl file) error {
 	st.logger.Printf("hashSearch(path=%s, len(sums)=%d)", fl.path, len(head.Sums))
-	f, err := os.Open(fl.path)
+	f, err := os.OpenFile(fl.path, os.O_RDONLY|nofollow.Maybe, 0)
 	if err != nil {
 		return err
 	}

rsyncd/sender.go

@@ -7,6 +7,7 @@ import (
 	"sort"
 
 	"github.com/gokrazy/rsync"
+	"github.com/gokrazy/rsync/internal/nofollow"
 	"github.com/gokrazy/rsync/internal/rsyncchecksum"
 	"github.com/gokrazy/rsync/internal/rsynccommon"
 	"github.com/mmcloughlin/md4"
@@ -150,7 +151,7 @@ func (st *sendTransfer) sendFile(fileIndex int32, fl file) error {
 	// increases throughput with “tridge” rsync as client by 50 Mbit/s.
 	const chunkSize = 256 * 1024
 
-	f, err := os.Open(fl.path)
+	f, err := os.OpenFile(fl.path, os.O_RDONLY|nofollow.Maybe, 0)
 	if err != nil {
 		return err
 	}