zip downloads: make zip entries relative to the current dir when possible

Signed-off-by: Nicola Murino <[email protected]>

go.mod

@@ -68,7 +68,7 @@ require (
 	golang.org/x/crypto v0.0.0-20220829220503-c86fa9a7ed90
 	golang.org/x/net v0.0.0-20220909164309-bea034e7d591
 	golang.org/x/oauth2 v0.0.0-20220909003341-f21342109be1
-	golang.org/x/sys v0.0.0-20220915200043-7b5979e65e41
+	golang.org/x/sys v0.0.0-20220919091848-fb04ddd9f9c8
 	golang.org/x/time v0.0.0-20220722155302-e5dcc9cfc0b9
 	google.golang.org/api v0.96.0
 	gopkg.in/natefinch/lumberjack.v2 v2.0.0
@@ -156,7 +156,7 @@ require (
 	golang.org/x/tools v0.1.12 // indirect
 	golang.org/x/xerrors v0.0.0-20220907171357-04be3eba64a2 // indirect
 	google.golang.org/appengine v1.6.7 // indirect
-	google.golang.org/genproto v0.0.0-20220916172020-2692e8806bfa // indirect
+	google.golang.org/genproto v0.0.0-20220919141832-68c03719ef51 // indirect
 	google.golang.org/grpc v1.49.0 // indirect
 	google.golang.org/protobuf v1.28.1 // indirect
 	gopkg.in/ini.v1 v1.67.0 // indirect

go.sum

@@ -980,8 +980,8 @@ golang.org/x/sys v0.0.0-20220610221304-9f5ed59c137d/go.mod h1:oPkhp1MJrh7nUepCBc
 golang.org/x/sys v0.0.0-20220704084225-05e143d24a9e/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/sys v0.0.0-20220728004956-3c1f35247d10/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/sys v0.0.0-20220811171246-fbc7d0a398ab/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
-golang.org/x/sys v0.0.0-20220915200043-7b5979e65e41 h1:ohgcoMbSofXygzo6AD2I1kz3BFmW1QArPYTtwEM3UXc=
-golang.org/x/sys v0.0.0-20220915200043-7b5979e65e41/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
+golang.org/x/sys v0.0.0-20220919091848-fb04ddd9f9c8 h1:h+EGohizhe9XlX18rfpa8k8RAc5XyaeamM+0VHRd4lc=
+golang.org/x/sys v0.0.0-20220919091848-fb04ddd9f9c8/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo=
 golang.org/x/term v0.0.0-20210927222741-03fcf44c2211 h1:JGgROgKl9N8DuW20oFS5gxc+lE67/N3FcwmBPMe7ArY=
 golang.org/x/term v0.0.0-20210927222741-03fcf44c2211/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8=
@@ -1229,8 +1229,8 @@ google.golang.org/genproto v0.0.0-20220523171625-347a074981d8/go.mod h1:RAyBrSAP
 google.golang.org/genproto v0.0.0-20220608133413-ed9918b62aac/go.mod h1:KEWEmljWE5zPzLBa/oHl6DaEt9LmfH6WtH1OHIvleBA=
 google.golang.org/genproto v0.0.0-20220616135557-88e70c0c3a90/go.mod h1:KEWEmljWE5zPzLBa/oHl6DaEt9LmfH6WtH1OHIvleBA=
 google.golang.org/genproto v0.0.0-20220624142145-8cd45d7dbd1f/go.mod h1:KEWEmljWE5zPzLBa/oHl6DaEt9LmfH6WtH1OHIvleBA=
-google.golang.org/genproto v0.0.0-20220916172020-2692e8806bfa h1:VWkrxnAx2C2hirAP+W5ADU7e/+93Yhk//ioKd2XFyDI=
-google.golang.org/genproto v0.0.0-20220916172020-2692e8806bfa/go.mod h1:0Nb8Qy+Sk5eDzHnzlStwW3itdNaWoZA5XeSG+R3JHSo=
+google.golang.org/genproto v0.0.0-20220919141832-68c03719ef51 h1:ucpgjuzWqWrj0NEwjUpsGTf2IGxyLtmuSk0oGgifjec=
+google.golang.org/genproto v0.0.0-20220919141832-68c03719ef51/go.mod h1:0Nb8Qy+Sk5eDzHnzlStwW3itdNaWoZA5XeSG+R3JHSo=
 google.golang.org/grpc v1.19.0/go.mod h1:mqu4LbDTu4XGKhr4mRzUsmM4RtVoemTSY81AxZiDr8c=
 google.golang.org/grpc v1.20.1/go.mod h1:10oTOabMzJvdu6/UiuZezV6QK5dSlG84ov/aaiqXj38=
 google.golang.org/grpc v1.21.1/go.mod h1:oYelfM1adQP15Ek0mdvEgi9Df8B9CZIaU1084ijfRaM=

internal/httpd/api_shares.go

@@ -271,13 +271,13 @@ func (s *httpdServer) downloadFromShare(w http.ResponseWriter, r *http.Request)
 
 	compress := true
 	var info os.FileInfo
-	if len(share.Paths) == 1 && r.URL.Query().Get("compress") == "false" {
+	if len(share.Paths) == 1 {
 		info, err = connection.Stat(share.Paths[0], 1)
 		if err != nil {
 			sendAPIResponse(w, r, err, "", getRespStatus(err))
 			return
 		}
-		if info.Mode().IsRegular() {
+		if info.Mode().IsRegular() && r.URL.Query().Get("compress") == "false" {
 			compress = false
 		}
 	}
@@ -289,9 +289,16 @@ func (s *httpdServer) downloadFromShare(w http.ResponseWriter, r *http.Request)
 			err = connection.GetReadQuotaExceededError()
 			connection.Log(logger.LevelInfo, "denying share read due to quota limits")
 			sendAPIResponse(w, r, err, "", getMappedStatusCode(err))
+			dataprovider.UpdateShareLastUse(&share, -1) //nolint:errcheck
+			return
+		}
+		baseDir := "/"
+		if info != nil && info.IsDir() {
+			baseDir = share.Paths[0]
+			share.Paths[0] = "/"
 		}
 		w.Header().Set("Content-Disposition", fmt.Sprintf("attachment; filename=\"share-%v.zip\"", share.Name))
-		renderCompressedFiles(w, connection, "/", share.Paths, &share)
+		renderCompressedFiles(w, connection, baseDir, share.Paths, &share)
 		return
 	}
 	if status, err := downloadFile(w, r, connection, share.Paths[0], info, false, &share); err != nil {

internal/httpd/api_utils.go

@@ -264,14 +264,19 @@ func addZipEntry(wr *zip.Writer, conn *Connection, entryPath, baseDir string) er
 		conn.Log(logger.LevelDebug, "unable to add zip entry %#v, stat error: %v", entryPath, err)
 		return err
 	}
+	entryName, err := getZipEntryName(entryPath, baseDir)
+	if err != nil {
+		conn.Log(logger.LevelError, "unable to get zip entry name: %v", err)
+		return err
+	}
 	if info.IsDir() {
-		_, err := wr.CreateHeader(&zip.FileHeader{
-			Name:     getZipEntryName(entryPath, baseDir) + "/",
+		_, err = wr.CreateHeader(&zip.FileHeader{
+			Name:     entryName + "/",
 			Method:   zip.Deflate,
 			Modified: info.ModTime(),
 		})
 		if err != nil {
-			conn.Log(logger.LevelDebug, "unable to create zip entry %#v: %v", entryPath, err)
+			conn.Log(logger.LevelError, "unable to create zip entry %#v: %v", entryPath, err)
 			return err
 		}
 		contents, err := conn.ReadDir(entryPath)
@@ -289,7 +294,7 @@ func addZipEntry(wr *zip.Writer, conn *Connection, entryPath, baseDir string) er
 	}
 	if !info.Mode().IsRegular() {
 		// we only allow regular files
-		conn.Log(logger.LevelDebug, "skipping zip entry for non regular file %#v", entryPath)
+		conn.Log(logger.LevelInfo, "skipping zip entry for non regular file %#v", entryPath)
 		return nil
 	}
 	reader, err := conn.getFileReader(entryPath, 0, http.MethodGet)
@@ -300,21 +305,24 @@ func addZipEntry(wr *zip.Writer, conn *Connection, entryPath, baseDir string) er
 	defer reader.Close()
 
 	f, err := wr.CreateHeader(&zip.FileHeader{
-		Name:     getZipEntryName(entryPath, baseDir),
+		Name:     entryName,
 		Method:   zip.Deflate,
 		Modified: info.ModTime(),
 	})
 	if err != nil {
-		conn.Log(logger.LevelDebug, "unable to create zip entry %#v: %v", entryPath, err)
+		conn.Log(logger.LevelError, "unable to create zip entry %#v: %v", entryPath, err)
 		return err
 	}
 	_, err = io.Copy(f, reader)
 	return err
 }
 
-func getZipEntryName(entryPath, baseDir string) string {
+func getZipEntryName(entryPath, baseDir string) (string, error) {
+	if !strings.HasPrefix(entryPath, baseDir) {
+		return "", fmt.Errorf("entry path %q is outside base dir %q", entryPath, baseDir)
+	}
 	entryPath = strings.TrimPrefix(entryPath, baseDir)
-	return strings.TrimPrefix(entryPath, "/")
+	return strings.TrimPrefix(entryPath, "/"), nil
 }
 
 func checkDownloadFileFromShare(share *dataprovider.Share, info os.FileInfo) error {

internal/httpd/httpd_test.go

@@ -11555,8 +11555,8 @@ func TestShareUsage(t *testing.T) {
 	user, _, err = httpdtest.UpdateUser(user, http.StatusOK, "")
 	assert.NoError(t, err)
 
-	share.Scope = dataprovider.ShareScopeReadWrite
-	share.Paths = []string{"/missing"}
+	share.Scope = dataprovider.ShareScopeRead
+	share.Paths = []string{"/missing1", "/missing2"}
 	err = dataprovider.UpdateShare(&share, user.Username, "")
 	assert.NoError(t, err)
 

internal/httpd/internal_test.go

@@ -1811,6 +1811,11 @@ func TestZipErrors(t *testing.T) {
 		assert.Contains(t, err.Error(), "write error")
 	}
 
+	err = addZipEntry(wr, connection, "/"+filepath.Base(testDir), path.Join("/", filepath.Base(testDir), "dir"))
+	if assert.Error(t, err) {
+		assert.Contains(t, err.Error(), "is outside base dir")
+	}
+
 	testFilePath := filepath.Join(testDir, "ziptest.zip")
 	err = os.WriteFile(testFilePath, util.GenerateRandomBytes(65535), os.ModePerm)
 	assert.NoError(t, err)

internal/util/util.go

@@ -191,7 +191,7 @@ func ByteCountIEC(b int64) string {
 }
 
 func byteCount(b int64, unit int64, maxPrecision bool) string {
-	if b <= 0 {
+	if b <= 0 && maxPrecision {
 		return strconv.FormatInt(b, 10)
 	}
 	if b < unit {