From b7a118401996089246fcc313e2e79f3eeed603bd Mon Sep 17 00:00:00 2001 From: Nick Craig-Wood Date: Mon, 27 Jul 2026 18:04:55 +0100 Subject: [PATCH] serve ftp: use constant time comparison for password check GHSA-mfvx-7rcj-9m5g The builtin authentication compared the configured username and password with ==, whose run time depends on how much of the value matches, giving a timing side-channel that could in principle help guess the password. serve sftp, serve s3 and the auth proxy already use subtle.ConstantTimeCompare so bring serve ftp in line with them. An empty configured password still accepts any password. Addresses GHSA-mfvx-7rcj-9m5g finding 4. --- cmd/serve/ftp/ftp.go | 9 ++++++++- cmd/serve/ftp/ftp_test.go | 30 ++++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/cmd/serve/ftp/ftp.go b/cmd/serve/ftp/ftp.go index e5f574ae1..6d15d4cfc 100644 --- a/cmd/serve/ftp/ftp.go +++ b/cmd/serve/ftp/ftp.go @@ -5,6 +5,7 @@ package ftp import ( "context" + "crypto/subtle" "errors" "fmt" "io" @@ -333,7 +334,13 @@ func (d *driver) CheckPasswd(sctx *ftp.Context, user, pass string) (ok bool, err d.userPass[user] = oPass d.userPassMu.Unlock() } else { - ok = d.opt.User == user && (d.opt.Pass == "" || d.opt.Pass == pass) + userOK := subtle.ConstantTimeCompare([]byte(d.opt.User), []byte(user)) + // No password configured means any password is accepted + passOK := 1 + if d.opt.Pass != "" { + passOK = subtle.ConstantTimeCompare([]byte(d.opt.Pass), []byte(pass)) + } + ok = (userOK & passOK) == 1 if !ok { fs.Infof(nil, "login failed: bad credentials") return false, nil diff --git a/cmd/serve/ftp/ftp_test.go b/cmd/serve/ftp/ftp_test.go index 83255fc77..df6655ac5 100644 --- a/cmd/serve/ftp/ftp_test.go +++ b/cmd/serve/ftp/ftp_test.go @@ -70,6 +70,36 @@ func TestFTP(t *testing.T) { servetest.Run(t, "ftp", start) } +// TestCheckPasswd checks the builtin authentication accepts only the +// configured credentials, with an empty configured password accepting any +// password. +func TestCheckPasswd(t *testing.T) { + for _, test := range []struct { + name string + optUser string + optPass string + user string + pass string + want bool + }{ + {name: "good", optUser: "user", optPass: "pass", user: "user", pass: "pass", want: true}, + {name: "bad-pass", optUser: "user", optPass: "pass", user: "user", pass: "PASS", want: false}, + {name: "bad-user", optUser: "user", optPass: "pass", user: "USER", pass: "pass", want: false}, + {name: "wrong-length-pass", optUser: "user", optPass: "pass", user: "user", pass: "pass2", want: false}, + {name: "empty-configured-pass", optUser: "user", optPass: "", user: "user", pass: "anything", want: true}, + {name: "empty-configured-pass-bad-user", optUser: "user", optPass: "", user: "USER", pass: "anything", want: false}, + } { + t.Run(test.name, func(t *testing.T) { + d := &driver{} + d.opt.User = test.optUser + d.opt.Pass = test.optPass + ok, err := d.CheckPasswd(nil, test.user, test.pass) + assert.NoError(t, err) + assert.Equal(t, test.want, ok) + }) + } +} + func TestRc(t *testing.T) { if israce.Enabled { t.Skip("Skipping under race detector as underlying library is racy")