use SafeJoin in the uploadHandler

Author: alessio-perugini

conn.go

@@ -140,7 +140,11 @@ func uploadHandler(c *gin.Context) {
 	}
 
 	for _, extraFile := range data.ExtraFiles {
-		path := filepath.Join(tmpdir, extraFile.Filename)
+		path, err := utilities.SafeJoin(tmpdir, extraFile.Filename)
+		if err != nil {
+			c.String(http.StatusBadRequest, err.Error())
+			return
+		}
 		filePaths = append(filePaths, path)
 		log.Printf("Saving %s on %s", extraFile.Filename, path)
 
@@ -150,7 +154,7 @@ func uploadHandler(c *gin.Context) {
 			return
 		}
 
-		err := os.WriteFile(path, extraFile.Hex, 0644)
+		err = os.WriteFile(path, extraFile.Hex, 0644)
 		if err != nil {
 			c.String(http.StatusBadRequest, err.Error())
 			return

main_test.go

@@ -26,6 +26,7 @@ import (
 	"path/filepath"
 	"testing"
 
+	"github.com/arduino/arduino-create-agent/upload"
 	"github.com/arduino/arduino-create-agent/config"
 	"github.com/arduino/arduino-create-agent/gen/tools"
 	v2 "github.com/arduino/arduino-create-agent/v2"
@@ -48,6 +49,42 @@ func TestValidSignatureKey(t *testing.T) {
 	require.NotNil(t, key)
 }
 
+func TestUploadHandlerAgainstEvilFileNames(t *testing.T) {
+	r := gin.New()
+	r.POST("/", uploadHandler)
+	ts := httptest.NewServer(r)
+
+	uploadEvilFileName := Upload{
+		Port:       "/dev/ttyACM0",
+		Board:      "arduino:avr:uno",
+		Extra:      upload.Extra{Network: true},
+		Hex:        []byte("test"),
+		Filename:   "../evil.txt",
+		ExtraFiles: []additionalFile{{Hex: []byte("test"), Filename: "../evil.txt"}},
+	}
+	uploadEvilExtraFile := Upload{
+		Port:       "/dev/ttyACM0",
+		Board:      "arduino:avr:uno",
+		Extra:      upload.Extra{Network: true},
+		Hex:        []byte("test"),
+		Filename:   "file.txt",
+		ExtraFiles: []additionalFile{{Hex: []byte("test"), Filename: "../evil.txt"}},
+	}
+
+	for _, request := range []Upload{uploadEvilFileName, uploadEvilExtraFile} {
+		payload, err := json.Marshal(request)
+		require.NoError(t, err)
+
+		resp, err := http.Post(ts.URL, "encoding/json", bytes.NewBuffer(payload))
+		require.NoError(t, err)
+		require.Equal(t, http.StatusBadRequest, resp.StatusCode)
+
+		body, err := io.ReadAll(resp.Body)
+		require.NoError(t, err)
+		require.Contains(t, string(body), "unsafe path join")
+	}
+}
+
 func TestInstallToolDifferentContentType(t *testing.T) {
 	r := gin.New()
 	goa := v2.Server(config.GetDataDir().String())