From 13bc190f8bdfa0b24a7773dda33c188403d0e17e Mon Sep 17 00:00:00 2001 From: Zack Date: Mon, 20 May 2024 08:08:38 -0700 Subject: [PATCH 1/3] ignore big file --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index 3a1d586da..b657493ec 100644 --- a/.gitignore +++ b/.gitignore @@ -9,3 +9,4 @@ croc-stdin* .idea/ .vscode/ +src/utils/bigfile.test From b05c3c8c42eb681f1586824e1887ea74845f6154 Mon Sep 17 00:00:00 2001 From: Zack Date: Mon, 20 May 2024 08:23:21 -0700 Subject: [PATCH 2/3] fix: client quits when discovering dangerous paths --- src/croc/croc.go | 12 ++++++++++++ src/utils/utils.go | 6 ++++++ 2 files changed, 18 insertions(+) diff --git a/src/croc/croc.go b/src/croc/croc.go index 24e414b5a..f3f689526 100644 --- a/src/croc/croc.go +++ b/src/croc/croc.go @@ -1092,6 +1092,18 @@ func (c *Client) processMessageFileInfo(m message.Message) (done bool, err error c.EmptyFoldersToTransfer = senderInfo.EmptyFoldersToTransfer c.TotalNumberFolders = senderInfo.TotalNumberFolders c.FilesToTransfer = senderInfo.FilesToTransfer + for i, fi := range c.FilesToTransfer { + // Issues #593 - sanitize the sender paths and prevent ".." from being used + c.FilesToTransfer[i].FolderRemote = filepath.Clean(fi.FolderRemote) + if strings.Contains(c.FilesToTransfer[i].FolderRemote, "..") { + return true, fmt.Errorf("invalid path detected: '%s'", fi.FolderRemote) + } + // Issues #593 - disallow specific folders like .ssh + if strings.Contains(c.FilesToTransfer[i].FolderRemote, ".ssh") { + return true, fmt.Errorf("invalid path detected: '%s'", fi.FolderRemote) + } + + } c.TotalNumberOfContents = 0 if c.FilesToTransfer != nil { c.TotalNumberOfContents += len(c.FilesToTransfer) diff --git a/src/utils/utils.go b/src/utils/utils.go index 75a1f1229..4b6cf8a89 100644 --- a/src/utils/utils.go +++ b/src/utils/utils.go @@ -438,6 +438,12 @@ func UnzipDirectory(destination string, source string) error { filePath := filepath.Join(destination, f.Name) fmt.Fprintf(os.Stderr, "\r\033[2K") fmt.Fprintf(os.Stderr, "\rUnzipping file %s", filePath) + // Issue #593 conceal path traversal vulnerability + // make sure the filepath does not have ".." + filePath = filepath.Clean(filePath) + if strings.Contains(filePath, "..") { + log.Fatalf("Invalid file path %s\n", filePath) + } if f.FileInfo().IsDir() { os.MkdirAll(filePath, os.ModePerm) continue From a591833dbf8a5c00920f7d6b0ebef59b4eb2c64b Mon Sep 17 00:00:00 2001 From: Zack Date: Mon, 20 May 2024 08:38:36 -0700 Subject: [PATCH 3/3] fix: filter escape sequences in filenames --- src/croc/croc.go | 4 ++++ src/utils/utils.go | 15 +++++++++++++++ 2 files changed, 19 insertions(+) diff --git a/src/croc/croc.go b/src/croc/croc.go index f3f689526..7badda777 100644 --- a/src/croc/croc.go +++ b/src/croc/croc.go @@ -1102,7 +1102,11 @@ func (c *Client) processMessageFileInfo(m message.Message) (done bool, err error if strings.Contains(c.FilesToTransfer[i].FolderRemote, ".ssh") { return true, fmt.Errorf("invalid path detected: '%s'", fi.FolderRemote) } + // Issue #595 - disallow filenames with anything but 0-9a-zA-Z.-_. and / characters + if !utils.ValidFileName(path.Join(c.FilesToTransfer[i].FolderRemote, fi.Name)) { + return true, fmt.Errorf("invalid filename detected: '%s'", fi.Name) + } } c.TotalNumberOfContents = 0 if c.FilesToTransfer != nil { diff --git a/src/utils/utils.go b/src/utils/utils.go index 4b6cf8a89..e117c8a26 100644 --- a/src/utils/utils.go +++ b/src/utils/utils.go @@ -473,3 +473,18 @@ func UnzipDirectory(destination string, source string) error { fmt.Fprintf(os.Stderr, "\n") return nil } + +// ValidFileName checks if a filename is valid +// and returns true only if it all of the characters are either +// 0-9, a-z, A-Z, ., _, -, space, or / +func ValidFileName(fname string) bool { + for _, r := range fname { + if !((r >= '0' && r <= '9') || + (r >= 'a' && r <= 'z') || + (r >= 'A' && r <= 'Z') || + r == '.' || r == '_' || r == '-' || r == ' ' || r == '/') { + return false + } + } + return true +}