shares: fix a directory share widened to the whole user filesystem

Downloading a public share limited to a single directory as a zip archive
rewrote share.Paths[0] to "/" so the entry names came out relative to that
directory. The value it overwrote is the policy every later public request is
authorized against: the browsable listing resolves the requested path under
Paths[0] and the upload handler derives its destination prefix from it.

The rewrite stays confined to the copy the handler holds only if the data
provider returns a share that owns its paths. The memory provider did not:
getACopy assigned the Paths slice by header, so the copy kept pointing at the
array stored in the provider map and the rewrite reached the stored share.
From that moment the link served the whole filesystem of the share owner. Any
recipient, with no account and nothing beyond the share URL, could list and
download every file the owner can reach, and a read-write share also accepted
uploads at the owner's root. The archive returned to the recipient who
triggered the change was still limited to the shared directory, so the
response revealed nothing. The SQL and bolt providers rebuild the slice while
decoding the stored row and were not affected.

The list of files to archive is now local to the handler, so the share is no
longer written to, and getACopy clones the paths, so a copy never shares state
with the provider.

The regression test downloads a share limited to one directory twice and
checks the archive, the stored paths, the browsable listing and a file outside
the share in between, with whichever data provider the test suite runs with.

Thanks to Yutaka Sasaki (@SAYUTIM) for the report.

Signed-off-by: Nicola Murino <nicola.murino@gmail.com>

internal/dataprovider/share.go

@@ -18,6 +18,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"net"
+	"slices"
 	"strconv"
 	"strings"
 	"time"
@@ -101,16 +102,13 @@ func (s *Share) GetSignature() string {
 }
 
 func (s *Share) getACopy() Share {
-	allowFrom := make([]string, len(s.AllowFrom))
-	copy(allowFrom, s.AllowFrom)
-
 	return Share{
 		ID:          s.ID,
 		ShareID:     s.ShareID,
 		Name:        s.Name,
 		Description: s.Description,
 		Scope:       s.Scope,
-		Paths:       s.Paths,
+		Paths:       slices.Clone(s.Paths),
 		Username:    s.Username,
 		CreatedAt:   s.CreatedAt,
 		UpdatedAt:   s.UpdatedAt,
@@ -119,7 +117,7 @@ func (s *Share) getACopy() Share {
 		Password:    s.Password,
 		MaxTokens:   s.MaxTokens,
 		UsedTokens:  s.UsedTokens,
-		AllowFrom:   allowFrom,
+		AllowFrom:   slices.Clone(s.AllowFrom),
 	}
 }
 

internal/httpd/api_shares.go

@@ -320,12 +320,14 @@ func (s *httpdServer) downloadFromShare(w http.ResponseWriter, r *http.Request)
 			return
 		}
 		baseDir := "/"
+		files := share.Paths
 		if info != nil && info.IsDir() {
+			// zip entry names are relative to the shared directory
 			baseDir = share.Paths[0]
-			share.Paths[0] = "/"
+			files = []string{"/"}
 		}
 		w.Header().Set("Content-Disposition", fmt.Sprintf("attachment; filename=\"share-%v.zip\"", share.Name))
-		renderCompressedFiles(w, connection, baseDir, share.Paths, &share)
+		renderCompressedFiles(w, connection, baseDir, files, &share)
 		return
 	}
 	if status, err := downloadFile(w, r, connection, share.Paths[0], info, false, &share); err != nil {

internal/httpd/httpd_test.go

@@ -15,6 +15,7 @@
 package httpd_test
 
 import (
+	"archive/zip"
 	"bytes"
 	"crypto/rand"
 	"encoding/base64"
@@ -15308,6 +15309,95 @@ func TestShareUsage(t *testing.T) {
 	executeRequest(req)
 }
 
+func TestShareDirDownloadPreservesPaths(t *testing.T) {
+	u := getTestUser()
+	u.Username = "share_zip_paths_" + xid.New().String()
+	user, _, err := httpdtest.AddUser(u, http.StatusCreated)
+	assert.NoError(t, err)
+
+	sharedDir := "shared"
+	sharedFileName := "visible.dat"
+	outsideFileName := "outside.dat"
+	err = os.MkdirAll(filepath.Join(user.GetHomeDir(), sharedDir), os.ModePerm)
+	assert.NoError(t, err)
+	err = createTestFile(filepath.Join(user.GetHomeDir(), sharedDir, sharedFileName), 32768)
+	assert.NoError(t, err)
+	err = createTestFile(filepath.Join(user.GetHomeDir(), outsideFileName), 32768)
+	assert.NoError(t, err)
+
+	token, err := getJWTAPIUserTokenFromTestServer(user.Username, defaultPassword)
+	assert.NoError(t, err)
+
+	share := dataprovider.Share{
+		Name:  "test_share_zip_paths",
+		Scope: dataprovider.ShareScopeRead,
+		Paths: []string{"/" + sharedDir},
+	}
+	asJSON, err := json.Marshal(share)
+	assert.NoError(t, err)
+	req, err := http.NewRequest(http.MethodPost, userSharesPath, bytes.NewBuffer(asJSON))
+	assert.NoError(t, err)
+	setBearerForReq(req, token)
+	rr := executeRequest(req)
+	checkResponseCode(t, http.StatusCreated, rr)
+	objectID := rr.Header().Get("X-Object-ID")
+	assert.NotEmpty(t, objectID)
+
+	getZipEntries := func() []string {
+		r, errReq := http.NewRequest(http.MethodGet, sharesPath+"/"+objectID, nil)
+		assert.NoError(t, errReq)
+		resp := executeRequest(r)
+		checkResponseCode(t, http.StatusOK, resp)
+		zipReader, errZip := zip.NewReader(bytes.NewReader(resp.Body.Bytes()), int64(resp.Body.Len()))
+		assert.NoError(t, errZip)
+		var entries []string
+		for _, f := range zipReader.File {
+			entries = append(entries, f.Name)
+		}
+		return entries
+	}
+	listShare := func() []string {
+		r, errReq := http.NewRequest(http.MethodGet, sharesPath+"/"+objectID+"/dirs", nil)
+		assert.NoError(t, errReq)
+		resp := executeRequest(r)
+		checkResponseCode(t, http.StatusOK, resp)
+		var contents []map[string]any
+		err := json.Unmarshal(resp.Body.Bytes(), &contents)
+		assert.NoError(t, err)
+		var names []string
+		for _, c := range contents {
+			names = append(names, c["name"].(string))
+		}
+		return names
+	}
+	// zip entry names are relative to the shared directory
+	assert.Equal(t, []string{"/", sharedFileName}, getZipEntries())
+	// downloading the whole share must not expand the shared paths
+	req, err = http.NewRequest(http.MethodGet, path.Join(userSharesPath, objectID), nil)
+	assert.NoError(t, err)
+	setBearerForReq(req, token)
+	rr = executeRequest(req)
+	checkResponseCode(t, http.StatusOK, rr)
+	var savedShare dataprovider.Share
+	err = json.Unmarshal(rr.Body.Bytes(), &savedShare)
+	assert.NoError(t, err)
+	assert.Equal(t, []string{"/" + sharedDir}, savedShare.Paths)
+	// browsing the share still returns the shared directory contents
+	assert.Equal(t, []string{sharedFileName}, listShare())
+	// files outside the shared directory remain unreachable
+	req, err = http.NewRequest(http.MethodGet, sharesPath+"/"+objectID+"/files?path=/"+outsideFileName, nil)
+	assert.NoError(t, err)
+	rr = executeRequest(req)
+	checkResponseCode(t, http.StatusNotFound, rr)
+	// a new download returns the same archive
+	assert.Equal(t, []string{"/", sharedFileName}, getZipEntries())
+
+	_, err = httpdtest.RemoveUser(user, http.StatusOK)
+	assert.NoError(t, err)
+	err = os.RemoveAll(user.GetHomeDir())
+	assert.NoError(t, err)
+}
+
 func TestSharePasswordPolicy(t *testing.T) {
 	g := getTestGroup()
 	g.UserSettings.Filters.PasswordStrength = 70