diff --git a/weed/sftpd/sftp_filer.go b/weed/sftpd/sftp_filer.go index 718290ea4..7eea186b0 100644 --- a/weed/sftpd/sftp_filer.go +++ b/weed/sftpd/sftp_filer.go @@ -8,6 +8,7 @@ import ( "fmt" "io" "net/http" + "net/url" "os" "path" "strings" @@ -322,7 +323,8 @@ func (fs *SftpServer) removeDir(absPath string) error { func (fs *SftpServer) putFile(filepath string, reader io.Reader, user *user.User) error { dir, filename := util.FullPath(filepath).DirAndName() - uploadUrl := fmt.Sprintf("http://%s%s", fs.filerAddr, filepath) + // Escape the path so a "?" in the filename cannot inject filer query commands like cp.from/mv.from. + uploadUrl := (&url.URL{Scheme: "http", Host: fs.filerAddr.ToHttpAddress(), Path: filepath}).String() // Let the global HTTP client normalize the scheme to https:// when TLS is configured normalizedUrl, err := util_http.NormalizeUrl(uploadUrl) if err != nil { diff --git a/weed/sftpd/sftp_filer_test.go b/weed/sftpd/sftp_filer_test.go new file mode 100644 index 000000000..0cab535ab --- /dev/null +++ b/weed/sftpd/sftp_filer_test.go @@ -0,0 +1,39 @@ +package sftpd + +import ( + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/seaweedfs/seaweedfs/weed/pb" +) + +// TestPutFileEscapesQueryInjection ensures a filename containing "?" cannot be +// reinterpreted by the filer as a query string that injects cp.from/mv.from +// commands, which would let an SFTP user escape their home directory. +func TestPutFileEscapesQueryInjection(t *testing.T) { + var gotPath, gotRawQuery string + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + gotPath = r.URL.Path + gotRawQuery = r.URL.RawQuery + if _, err := w.Write([]byte(`{}`)); err != nil { + t.Errorf("write response: %v", err) + } + })) + defer ts.Close() + + fs := &SftpServer{filerAddr: pb.ServerAddress(strings.TrimPrefix(ts.URL, "http://"))} + + malicious := "/home/alice/steal?cp.from=/home/bob/secret.txt" + if err := fs.putFile(malicious, strings.NewReader("dummy"), nil); err != nil { + t.Fatalf("putFile: %v", err) + } + + if gotRawQuery != "" { + t.Errorf("filename leaked into query string: RawQuery=%q", gotRawQuery) + } + if gotPath != malicious { + t.Errorf("path not delivered literally: got %q, want %q", gotPath, malicious) + } +}