mirror of
https://github.com/seaweedfs/seaweedfs.git
synced 2026-08-15 19:56:39 +00:00
sftp: url-encode the upload path (#10758)
sftp: url-encode the upload path so filenames can't inject filer query commands
The SFTP put handler concatenated the user-controlled filename straight into
the filer upload URL, so a name containing "?" was parsed as a query string.
Build the URL via url.URL{Path: ...} so "?" becomes %3F and stays a literal
path character.
This commit is contained in:
@@ -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 {
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user