From 57639c4adf7e70100b086010d2e87c1ee30b62e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Roth?= Date: Thu, 10 Oct 2024 18:54:03 +0200 Subject: [PATCH 1/3] Sanitize path api params - fix path traversal complains by CodeQL --- api/files.go | 13 ++++++++----- api/publish.go | 16 ++++++++++------ api/repos.go | 8 ++++---- utils/utils.go | 8 ++++++++ 4 files changed, 30 insertions(+), 15 deletions(-) diff --git a/api/files.go b/api/files.go index bd9296cc..02f8d8cf 100644 --- a/api/files.go +++ b/api/files.go @@ -8,6 +8,7 @@ import ( "strings" "sync" + "github.com/aptly-dev/aptly/utils" "github.com/gin-gonic/gin" "github.com/saracen/walker" ) @@ -72,7 +73,7 @@ func apiFilesUpload(c *gin.Context) { return } - path := filepath.Join(context.UploadPath(), c.Params.ByName("dir")) + path := filepath.Join(context.UploadPath(), utils.PathSanitize(c.Params.ByName("dir"))) err := os.MkdirAll(path, 0777) if err != nil { @@ -128,7 +129,7 @@ func apiFilesListFiles(c *gin.Context) { list := []string{} listLock := &sync.Mutex{} - root := filepath.Join(context.UploadPath(), c.Params.ByName("dir")) + root := filepath.Join(context.UploadPath(), utils.PathSanitize(c.Params.ByName("dir"))) err := filepath.Walk(root, func(path string, _ os.FileInfo, err error) error { if err != nil { @@ -164,7 +165,7 @@ func apiFilesDeleteDir(c *gin.Context) { return } - err := os.RemoveAll(filepath.Join(context.UploadPath(), c.Params.ByName("dir"))) + err := os.RemoveAll(filepath.Join(context.UploadPath(), utils.PathSanitize(c.Params.ByName("dir")))) if err != nil { AbortWithJSONError(c, 500, err) return @@ -179,12 +180,14 @@ func apiFilesDeleteFile(c *gin.Context) { return } - if !verifyPath(c.Params.ByName("name")) { + dir := utils.PathSanitize(c.Params.ByName("dir")) + name := utils.PathSanitize(c.Params.ByName("name")) + if !verifyPath(name) { AbortWithJSONError(c, 400, fmt.Errorf("wrong file")) return } - err := os.Remove(filepath.Join(context.UploadPath(), c.Params.ByName("dir"), c.Params.ByName("name"))) + err := os.Remove(filepath.Join(context.UploadPath(), dir, name)) if err != nil { if err1, ok := err.(*os.PathError); !ok || !os.IsNotExist(err1.Err) { AbortWithJSONError(c, 500, err) diff --git a/api/publish.go b/api/publish.go index a434d90c..2d9dee79 100644 --- a/api/publish.go +++ b/api/publish.go @@ -9,6 +9,7 @@ import ( "github.com/aptly-dev/aptly/deb" "github.com/aptly-dev/aptly/pgp" "github.com/aptly-dev/aptly/task" + "github.com/aptly-dev/aptly/utils" "github.com/gin-gonic/gin" ) @@ -43,9 +44,10 @@ func getSigner(options *SigningOptions) (pgp.Signer, error) { return signer, nil } -// Replace '_' with '/' and double '__' with single '_' -func parseEscapedPath(path string) string { +// Replace '_' with '/' and double '__' with single '_', PathSanitize +func slashEscape(path string) string { result := strings.Replace(strings.Replace(path, "_", "/", -1), "//", "_", -1) + result = utils.PathSanitize(result) if result == "" { result = "." } @@ -86,7 +88,7 @@ func apiPublishList(c *gin.Context) { // POST /publish/:prefix func apiPublishRepoOrSnapshot(c *gin.Context) { - param := parseEscapedPath(c.Params.ByName("prefix")) + param := slashEscape(c.Params.ByName("prefix")) storage, prefix := deb.ParsePrefix(param) var b struct { @@ -113,6 +115,8 @@ func apiPublishRepoOrSnapshot(c *gin.Context) { return } + b.Distribution = utils.PathSanitize(b.Distribution) + signer, err := getSigner(&b.Signing) if err != nil { AbortWithJSONError(c, 500, fmt.Errorf("unable to initialize GPG signer: %s", err)) @@ -248,9 +252,9 @@ func apiPublishRepoOrSnapshot(c *gin.Context) { // PUT /publish/:prefix/:distribution func apiPublishUpdateSwitch(c *gin.Context) { - param := parseEscapedPath(c.Params.ByName("prefix")) + param := slashEscape(c.Params.ByName("prefix")) storage, prefix := deb.ParsePrefix(param) - distribution := c.Params.ByName("distribution") + distribution := utils.PathSanitize(c.Params.ByName("distribution")) var b struct { ForceOverwrite bool @@ -373,7 +377,7 @@ func apiPublishDrop(c *gin.Context) { force := c.Request.URL.Query().Get("force") == "1" skipCleanup := c.Request.URL.Query().Get("SkipCleanup") == "1" - param := parseEscapedPath(c.Params.ByName("prefix")) + param := slashEscape(c.Params.ByName("prefix")) storage, prefix := deb.ParsePrefix(param) distribution := c.Params.ByName("distribution") diff --git a/api/repos.go b/api/repos.go index 81425f99..91285beb 100644 --- a/api/repos.go +++ b/api/repos.go @@ -343,8 +343,8 @@ func apiReposPackageFromDir(c *gin.Context) { return } - dirParam := c.Params.ByName("dir") - fileParam := c.Params.ByName("file") + dirParam := utils.PathSanitize(c.Params.ByName("dir")) + fileParam := utils.PathSanitize(c.Params.ByName("file")) if fileParam != "" && !verifyPath(fileParam) { AbortWithJSONError(c, 400, fmt.Errorf("wrong file")) return @@ -620,8 +620,8 @@ func apiReposIncludePackageFromDir(c *gin.Context) { var sources []string var taskName string - dirParam := c.Params.ByName("dir") - fileParam := c.Params.ByName("file") + dirParam := utils.PathSanitize(c.Params.ByName("dir")) + fileParam := utils.PathSanitize(c.Params.ByName("file")) if fileParam != "" && !verifyPath(fileParam) { AbortWithJSONError(c, 400, fmt.Errorf("wrong file")) return diff --git a/utils/utils.go b/utils/utils.go index 4d4734fc..0a026b77 100644 --- a/utils/utils.go +++ b/utils/utils.go @@ -4,6 +4,7 @@ package utils import ( "fmt" "os" + "strings" "golang.org/x/sys/unix" ) @@ -22,3 +23,10 @@ func DirIsAccessible(filename string) error { } return nil } + +// Remove leading '/', remove '..' +func PathSanitize(path string) (result string) { + result = strings.Replace(path, "..", "", -1) + result = strings.TrimLeft(result, "/") + return +} From 77429804263c6d9a52e27a84e7b341f230404195 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Roth?= Date: Thu, 10 Oct 2024 23:05:17 +0200 Subject: [PATCH 2/3] use specific go version As of Go 1.21, toolchain versions must use the 1.N.P syntax. --- go.mod | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go.mod b/go.mod index 7585ce1f..d053f1df 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module github.com/aptly-dev/aptly -go 1.22 +go 1.22.7 require ( github.com/AlekSi/pointer v1.1.0 From cefc09a41b3aefa7067275a360a0ea5321e3d7be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Roth?= Date: Fri, 11 Oct 2024 13:37:33 +0200 Subject: [PATCH 3/3] more sanitize --- api/files.go | 10 +++++----- api/gpg.go | 5 +++++ api/publish.go | 8 ++++---- api/repos.go | 8 ++++---- utils/utils.go | 6 ++++-- 5 files changed, 22 insertions(+), 15 deletions(-) diff --git a/api/files.go b/api/files.go index 02f8d8cf..7ae682f8 100644 --- a/api/files.go +++ b/api/files.go @@ -73,7 +73,7 @@ func apiFilesUpload(c *gin.Context) { return } - path := filepath.Join(context.UploadPath(), utils.PathSanitize(c.Params.ByName("dir"))) + path := filepath.Join(context.UploadPath(), utils.SanitizePath(c.Params.ByName("dir"))) err := os.MkdirAll(path, 0777) if err != nil { @@ -129,7 +129,7 @@ func apiFilesListFiles(c *gin.Context) { list := []string{} listLock := &sync.Mutex{} - root := filepath.Join(context.UploadPath(), utils.PathSanitize(c.Params.ByName("dir"))) + root := filepath.Join(context.UploadPath(), utils.SanitizePath(c.Params.ByName("dir"))) err := filepath.Walk(root, func(path string, _ os.FileInfo, err error) error { if err != nil { @@ -165,7 +165,7 @@ func apiFilesDeleteDir(c *gin.Context) { return } - err := os.RemoveAll(filepath.Join(context.UploadPath(), utils.PathSanitize(c.Params.ByName("dir")))) + err := os.RemoveAll(filepath.Join(context.UploadPath(), utils.SanitizePath(c.Params.ByName("dir")))) if err != nil { AbortWithJSONError(c, 500, err) return @@ -180,8 +180,8 @@ func apiFilesDeleteFile(c *gin.Context) { return } - dir := utils.PathSanitize(c.Params.ByName("dir")) - name := utils.PathSanitize(c.Params.ByName("name")) + dir := utils.SanitizePath(c.Params.ByName("dir")) + name := utils.SanitizePath(c.Params.ByName("name")) if !verifyPath(name) { AbortWithJSONError(c, 400, fmt.Errorf("wrong file")) return diff --git a/api/gpg.go b/api/gpg.go index 605b6c1d..72e938d2 100644 --- a/api/gpg.go +++ b/api/gpg.go @@ -8,6 +8,7 @@ import ( "strings" "github.com/aptly-dev/aptly/pgp" + "github.com/aptly-dev/aptly/utils" "github.com/gin-gonic/gin" ) @@ -23,6 +24,10 @@ func apiGPGAddKey(c *gin.Context) { if c.Bind(&b) != nil { return } + b.Keyserver = utils.SanitizePath(b.Keyserver) + b.GpgKeyID = utils.SanitizePath(b.GpgKeyID) + b.GpgKeyArmor = utils.SanitizePath(b.GpgKeyArmor) + // b.Keyring can be an absolute path var err error args := []string{"--no-default-keyring", "--allow-non-selfsigned-uid"} diff --git a/api/publish.go b/api/publish.go index 2d9dee79..9873cd58 100644 --- a/api/publish.go +++ b/api/publish.go @@ -44,10 +44,10 @@ func getSigner(options *SigningOptions) (pgp.Signer, error) { return signer, nil } -// Replace '_' with '/' and double '__' with single '_', PathSanitize +// Replace '_' with '/' and double '__' with single '_', SanitizePath func slashEscape(path string) string { result := strings.Replace(strings.Replace(path, "_", "/", -1), "//", "_", -1) - result = utils.PathSanitize(result) + result = utils.SanitizePath(result) if result == "" { result = "." } @@ -115,7 +115,7 @@ func apiPublishRepoOrSnapshot(c *gin.Context) { return } - b.Distribution = utils.PathSanitize(b.Distribution) + b.Distribution = utils.SanitizePath(b.Distribution) signer, err := getSigner(&b.Signing) if err != nil { @@ -254,7 +254,7 @@ func apiPublishRepoOrSnapshot(c *gin.Context) { func apiPublishUpdateSwitch(c *gin.Context) { param := slashEscape(c.Params.ByName("prefix")) storage, prefix := deb.ParsePrefix(param) - distribution := utils.PathSanitize(c.Params.ByName("distribution")) + distribution := utils.SanitizePath(c.Params.ByName("distribution")) var b struct { ForceOverwrite bool diff --git a/api/repos.go b/api/repos.go index 91285beb..bc55dd9f 100644 --- a/api/repos.go +++ b/api/repos.go @@ -343,8 +343,8 @@ func apiReposPackageFromDir(c *gin.Context) { return } - dirParam := utils.PathSanitize(c.Params.ByName("dir")) - fileParam := utils.PathSanitize(c.Params.ByName("file")) + dirParam := utils.SanitizePath(c.Params.ByName("dir")) + fileParam := utils.SanitizePath(c.Params.ByName("file")) if fileParam != "" && !verifyPath(fileParam) { AbortWithJSONError(c, 400, fmt.Errorf("wrong file")) return @@ -620,8 +620,8 @@ func apiReposIncludePackageFromDir(c *gin.Context) { var sources []string var taskName string - dirParam := utils.PathSanitize(c.Params.ByName("dir")) - fileParam := utils.PathSanitize(c.Params.ByName("file")) + dirParam := utils.SanitizePath(c.Params.ByName("dir")) + fileParam := utils.SanitizePath(c.Params.ByName("file")) if fileParam != "" && !verifyPath(fileParam) { AbortWithJSONError(c, 400, fmt.Errorf("wrong file")) return diff --git a/utils/utils.go b/utils/utils.go index 0a026b77..eb9677d3 100644 --- a/utils/utils.go +++ b/utils/utils.go @@ -24,9 +24,11 @@ func DirIsAccessible(filename string) error { return nil } -// Remove leading '/', remove '..' -func PathSanitize(path string) (result string) { +// Remove leading '/', remove '..', '$' and '`' +func SanitizePath(path string) (result string) { result = strings.Replace(path, "..", "", -1) + result = strings.Replace(result, "$", "", -1) + result = strings.Replace(result, "`", "", -1) result = strings.TrimLeft(result, "/") return }