Skip to content

Commit

Permalink
use SafeJoin in the uploadHandler
Browse files Browse the repository at this point in the history
  • Loading branch information
alessio-perugini committed Sep 4, 2023
1 parent b2e6824 commit 415f238
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 2 deletions.
8 changes: 6 additions & 2 deletions conn.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand All @@ -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
Expand Down
43 changes: 43 additions & 0 deletions main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,18 @@
package main

import (
"bytes"
"crypto/x509"
"encoding/json"
"encoding/pem"
"io"
"net/http"
"net/http/httptest"
"path/filepath"
"testing"

"github.com/arduino/arduino-create-agent/upload"
"github.com/gin-gonic/gin"
"github.com/stretchr/testify/require"
)

Expand All @@ -38,3 +45,39 @@ func TestValidSignatureKey(t *testing.T) {
require.NoError(t, err)
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")
}
}

0 comments on commit 415f238

Please sign in to comment.