From 39abf24708870cec71a84c01063e647b859b2b67 Mon Sep 17 00:00:00 2001 From: JoramWilander Date: Tue, 21 Jul 2015 15:18:17 -0400 Subject: added sanitization to filenames to remove the possibility of relative paths --- api/file.go | 8 +++++--- api/file_test.go | 8 +++++++- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/api/file.go b/api/file.go index 2abaca709..1dd179422 100644 --- a/api/file.go +++ b/api/file.go @@ -89,9 +89,11 @@ func uploadFile(c *Context, w http.ResponseWriter, r *http.Request) { buf := bytes.NewBuffer(nil) io.Copy(buf, file) + filename := filepath.Base(files[i].Filename) + uid := model.NewId() - path := "teams/" + c.Session.TeamId + "/channels/" + channelId + "/users/" + c.Session.UserId + "/" + uid + "/" + files[i].Filename + path := "teams/" + c.Session.TeamId + "/channels/" + channelId + "/users/" + c.Session.UserId + "/" + uid + "/" + filename if err := writeFile(buf.Bytes(), path); err != nil { c.Err = err @@ -99,11 +101,11 @@ func uploadFile(c *Context, w http.ResponseWriter, r *http.Request) { } if model.IsFileExtImage(filepath.Ext(files[i].Filename)) { - imageNameList = append(imageNameList, uid+"/"+files[i].Filename) + imageNameList = append(imageNameList, uid+"/"+filename) imageDataList = append(imageDataList, buf.Bytes()) } - encName := utils.UrlEncode(files[i].Filename) + encName := utils.UrlEncode(filename) fileUrl := "/" + channelId + "/" + c.Session.UserId + "/" + uid + "/" + encName resStruct.Filenames = append(resStruct.Filenames, fileUrl) diff --git a/api/file_test.go b/api/file_test.go index d5817234d..3f414d768 100644 --- a/api/file_test.go +++ b/api/file_test.go @@ -38,7 +38,7 @@ func TestUploadFile(t *testing.T) { body := &bytes.Buffer{} writer := multipart.NewWriter(body) - part, err := writer.CreateFormFile("files", "test.png") + part, err := writer.CreateFormFile("files", "../test.png") if err != nil { t.Fatal(err) } @@ -75,6 +75,9 @@ func TestUploadFile(t *testing.T) { filenames := strings.Split(resp.Data.(*model.FileUploadResponse).Filenames[0], "/") filename := filenames[len(filenames)-2] + "/" + filenames[len(filenames)-1] + if strings.Contains(filename, "../") { + t.Fatal("relative path should have been sanitized out") + } fileId := strings.Split(filename, ".")[0] var auth aws.Auth @@ -104,6 +107,9 @@ func TestUploadFile(t *testing.T) { } else if utils.Cfg.ServiceSettings.UseLocalStorage && len(utils.Cfg.ServiceSettings.StorageDirectory) > 0 { filenames := strings.Split(resp.Data.(*model.FileUploadResponse).Filenames[0], "/") filename := filenames[len(filenames)-2] + "/" + filenames[len(filenames)-1] + if strings.Contains(filename, "../") { + t.Fatal("relative path should have been sanitized out") + } fileId := strings.Split(filename, ".")[0] // wait a bit for files to ready -- cgit v1.2.3-1-g7c22