From 1df2b70753286c1dfe8366078cbedfdf7f96472c Mon Sep 17 00:00:00 2001 From: Nick Craig-Wood Date: Mon, 20 Jul 2026 13:10:45 +0100 Subject: [PATCH] ftp: fix ftp command injection when encoding doesn't include CRLF GHSA-8c48-q9wj-3w37 CVE-PENDING The FTP control channel is line oriented and the ftp library writes command arguments (paths) straight onto it without escaping, so a filename containing CR/LF can inject an independent FTP command. This fix makes sure CR/LF are therefore always encoded to safe symbols regardless of the configured encoding, which is what the default encoding already does. --- backend/ftp/ftp.go | 13 +++++++++++++ backend/ftp/ftp_internal_test.go | 23 +++++++++++++++++++++++ 2 files changed, 36 insertions(+) diff --git a/backend/ftp/ftp.go b/backend/ftp/ftp.go index ce90d3f0f..7dcbdeb62 100644 --- a/backend/ftp/ftp.go +++ b/backend/ftp/ftp.go @@ -637,6 +637,18 @@ func (f *Fs) drainPool(ctx context.Context) (err error) { return err } +// commandEncoding hardens the user-configured filename encoding so that it +// can never restore a raw CR or LF. +// +// The FTP control channel is line oriented and the ftp library writes command +// arguments (paths) straight onto it without escaping, so a filename +// containing CR/LF would otherwise be able to inject an independent FTP +// command. CR/LF are therefore always encoded to safe symbols regardless of +// the configured encoding, which is what the default encoding already does. +func commandEncoding(enc encoder.MultiEncoder) encoder.MultiEncoder { + return enc | encoder.EncodeCrLf +} + // NewFs constructs an Fs from the path, container:path func NewFs(ctx context.Context, name, root string, m configmap.Mapper) (ff fs.Fs, err error) { // defer fs.Trace(nil, "name=%q, root=%q", name, root)("fs=%v, err=%v", &ff, &err) @@ -646,6 +658,7 @@ func NewFs(ctx context.Context, name, root string, m configmap.Mapper) (ff fs.Fs if err != nil { return nil, err } + opt.Enc = commandEncoding(opt.Enc) pass := "" if opt.AskPassword && opt.Pass == "" { pass = config.GetPassword("FTP server password") diff --git a/backend/ftp/ftp_internal_test.go b/backend/ftp/ftp_internal_test.go index d4c153b4f..187f94036 100644 --- a/backend/ftp/ftp_internal_test.go +++ b/backend/ftp/ftp_internal_test.go @@ -12,11 +12,34 @@ import ( "github.com/rclone/rclone/fs/object" "github.com/rclone/rclone/fstest" "github.com/rclone/rclone/fstest/fstests" + "github.com/rclone/rclone/lib/encoder" "github.com/rclone/rclone/lib/readers" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) +// A filename containing raw CR/LF must never survive encoding into an FTP +// command argument, whatever encoding the user configured, otherwise it could +// inject an independent FTP command onto the line-oriented control channel. +func TestCommandEncodingNeutralisesCRLF(t *testing.T) { + // Internal Standard-encoded name as produced by e.g. a Unix local + // source, carrying raw CR/LF. + for _, std := range []string{ + "victim\r\nDELE other-secret\r\nNOOP", + "victim␍␊DELE other-secret␍␊NOOP", + } { + for _, configured := range []encoder.MultiEncoder{ + encoder.EncodeSlash, // "Slash" - omits Ctl and CrLf + encoder.EncodeZero, // "None" + encoder.Display | encoder.EncodeRightSpace, // the FTP default + } { + got := commandEncoding(configured).FromStandardPath(std) + assert.NotContains(t, got, "\r", "encoding %v leaked raw CR: %q", configured, got) + assert.NotContains(t, got, "\n", "encoding %v leaked raw LF: %q", configured, got) + } + } +} + type settings map[string]any func deriveFs(ctx context.Context, t *testing.T, f fs.Fs, opts settings) fs.Fs {