added sanitization to filenames to remove the possibility of relative paths

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