From f19691250ddb15c0f2a441ac507197479a6e8995 Mon Sep 17 00:00:00 2001 From: Nicola Murino Date: Mon, 19 Sep 2022 17:06:42 +0200 Subject: [PATCH] zip downloads: make zip entries relative to the current dir when possible Signed-off-by: Nicola Murino --- go.mod | 4 ++-- go.sum | 8 ++++---- internal/httpd/api_shares.go | 13 ++++++++++--- internal/httpd/api_utils.go | 24 ++++++++++++++++-------- internal/httpd/httpd_test.go | 4 ++-- internal/httpd/internal_test.go | 5 +++++ internal/util/util.go | 2 +- 7 files changed, 40 insertions(+), 20 deletions(-) diff --git a/go.mod b/go.mod index c21c72ae..0869d0f3 100644 --- a/go.mod +++ b/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 diff --git a/go.sum b/go.sum index 623f002a..0376d095 100644 --- a/go.sum +++ b/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= diff --git a/internal/httpd/api_shares.go b/internal/httpd/api_shares.go index 042d59bf..9309c353 100644 --- a/internal/httpd/api_shares.go +++ b/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 { diff --git a/internal/httpd/api_utils.go b/internal/httpd/api_utils.go index 2c4a4988..4783ce05 100644 --- a/internal/httpd/api_utils.go +++ b/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 { diff --git a/internal/httpd/httpd_test.go b/internal/httpd/httpd_test.go index 8525a1dc..f01523c9 100644 --- a/internal/httpd/httpd_test.go +++ b/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) diff --git a/internal/httpd/internal_test.go b/internal/httpd/internal_test.go index caa7170b..8ebe9f28 100644 --- a/internal/httpd/internal_test.go +++ b/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) diff --git a/internal/util/util.go b/internal/util/util.go index 56491918..6ebbd334 100644 --- a/internal/util/util.go +++ b/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 {