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.
This commit is contained in:
@@ -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")
|
||||
|
||||
@@ -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 {
|
||||
|
||||
Reference in New Issue
Block a user