CVE-2023-22726: Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal') (CWE-22)
value: act is a project which allows for local running of github actions. The artifact server that stores artifacts from Github Action runs does not sanitize path inputs. This allows an attacker to download and overwrite arbitrary files on the host from a Github Action. This issue may lead to privilege escalation. The /upload endpoint is vulnerable to path traversal as filepath is user controlled, and ultimately flows into os.Mkdir and os.Open. The /artifact endpoint is vulnerable to path traversal as the path is variable is user controlled, and the specified file is ultimately returned by the server. This has been addressed in version 0.2.40. Users are advised to upgrade. Users unable to upgrade may, during implementation of Open and OpenAtEnd for FS, ensure to use ValidPath() to check against path traversal or clean the user-provided paths manually.
CWE description: The product uses external input to construct a pathname that is intended to identify a file or directory that is located underneath a restricted parent directory, but the product does not properly neutralize special elements within the pathname that can cause the pathname to resolve to a location that is outside of the restricted directory.
CVSS v3 base score: 8.8
Published: 2023-01-20T22:15Z
Repository: https://github.com/nektos/act
Language: Go
Files changed: .vscode/settings.json, pkg/artifacts/server.go, pkg/artifacts/server_test.go, pkg/artifacts/testdata/GHSL-2023-004/artifacts.yml
Commit date: 2023-01-16 21:01:54 +0000
Commit message: fix: update artifact server to address GHSL-2023-004 (#1565)


Diff: diff --git a/.vscode/settings.json b/.vscode/settings.json
index 8f4495d..3e305ae 100644
--- a/.vscode/settings.json
+++ b/.vscode/settings.json
@@ -1,8 +1,9 @@
 {
   "go.lintTool": "golangci-lint",
   "go.lintFlags": ["--fix"],
+  "go.testTimeout": "300s",
   "[json]": {
     "editor.defaultFormatter": "esbenp.prettier-vscode"
   },
   "[markdown]": {
     "editor.defaultFormatter": "esbenp.prettier-vscode"
diff --git a/pkg/artifacts/server.go b/pkg/artifacts/server.go
index 9b11457..7c3df56 100644
--- a/pkg/artifacts/server.go
+++ b/pkg/artifacts/server.go
@@ -7,11 +7,10 @@ import (
 	"fmt"
 	"io"
 	"io/fs"
 	"net/http"
 	"os"
-	"path"
 	"path/filepath"
 	"strings"
 	"time"
 
 	"github.com/julienschmidt/httprouter"
@@ -44,48 +43,57 @@ type ContainerItemResponse struct {
 
 type ResponseMessage struct {
 	Message string `json:"message"`
 }
 
-type MkdirFS interface {
-	fs.FS
-	MkdirAll(path string, perm fs.FileMode) error
-	Open(name string) (fs.File, error)
-	OpenAtEnd(name string) (fs.File, error)
+type WritableFile interface {
+	io.WriteCloser
 }
 
-type MkdirFsImpl struct {
-	dir string
-	fs.FS
+type WriteFS interface {
+	OpenWritable(name string) (WritableFile, error)
+	OpenAppendable(name string) (WritableFile, error)
 }
 
-func (fsys MkdirFsImpl) MkdirAll(path string, perm fs.FileMode) error {
-	return os.MkdirAll(fsys.dir+"/"+path, perm)
+type readWriteFSImpl struct {
 }
 
-func (fsys MkdirFsImpl) Open(name string) (fs.File, error) {
-	return os.OpenFile(fsys.dir+"/"+name, os.O_CREATE|os.O_RDWR|os.O_TRUNC, 0644)
+func (fwfs readWriteFSImpl) Open(name string) (fs.File, error) {
+	return os.Open(name)
 }
 
-func (fsys MkdirFsImpl) OpenAtEnd(name string) (fs.File, error) {
-	file, err := os.OpenFile(fsys.dir+"/"+name, os.O_CREATE|os.O_RDWR, 0644)
+func (fwfs readWriteFSImpl) OpenWritable(name string) (WritableFile, error) {
+	if err := os.MkdirAll(filepath.Dir(name), os.ModePerm); err != nil {
+		return nil, err
+	}
+	return os.OpenFile(name, os.O_CREATE|os.O_RDWR|os.O_TRUNC, 0644)
+}
+
+func (fwfs readWriteFSImpl) OpenAppendable(name string) (WritableFile, error) {
+	if err := os.MkdirAll(filepath.Dir(name), os.ModePerm); err != nil {
+		return nil, err
+	}
+	file, err := os.OpenFile(name, os.O_CREATE|os.O_RDWR, 0644)
 
 	if err != nil {
 		return nil, err
 	}
 
 	_, err = file.Seek(0, os.SEEK_END)
 	if err != nil {
 		return nil, err
 	}
-
 	return file, nil
 }
 
 var gzipExtension = ".gz__"
 
-func uploads(router *httprouter.Router, fsys MkdirFS) {
+func safeResolve(baseDir string, relPath string) string {
+	return filepath.Join(baseDir, filepath.Clean(filepath.Join(string(os.PathSeparator), relPath)))
+}
+
+func uploads(router *httprouter.Router, baseDir string, fsys WriteFS) {
 	router.POST("/_apis/pipelines/workflows/:runId/artifacts", func(w http.ResponseWriter, req *http.Request, params httprouter.Params) {
 		runID := params.ByName("runId")
 
 		json, err := json.Marshal(FileContainerResourceURL{
 			FileContainerResourceURL: fmt.Sprintf("http://%s/upload/%s", req.Host, runID),
@@ -106,23 +114,19 @@ func uploads(router *httprouter.Router, fsys MkdirFS) {
 
 		if req.Header.Get("Content-Encoding") == "gzip" {
 			itemPath += gzipExtension
 		}
 
-		filePath := fmt.Sprintf("%s/%s", runID, itemPath)
+		safeRunPath := safeResolve(baseDir, runID)
+		safePath := safeResolve(safeRunPath, itemPath)
 
-		err := fsys.MkdirAll(path.Dir(filePath), os.ModePerm)
-		if err != nil {
-			panic(err)
-		}
-
-		file, err := func() (fs.File, error) {
+		file, err := func() (WritableFile, error) {
 			contentRange := req.Header.Get("Content-Range")
 			if contentRange != "" && !strings.HasPrefix(contentRange, "bytes 0-") {
-				return fsys.OpenAtEnd(filePath)
+				return fsys.OpenAppendable(safePath)
 			}
-			return fsys.Open(filePath)
+			return fsys.OpenWritable(safePath)
 		}()
 
 		if err != nil {
 			panic(err)
 		}
@@ -168,15 +172,17 @@ func uploads(router *httprouter.Router, fsys MkdirFS) {
 			panic(err)
 		}
 	})
 }
 
-func downloads(router *httprouter.Router, fsys fs.FS) {
+func downloads(router *httprouter.Router, baseDir string, fsys fs.FS) {
 	router.GET("/_apis/pipelines/workflows/:runId/artifacts", func(w http.ResponseWriter, req *http.Request, params httprouter.Params) {
 		runID := params.ByName("runId")
 
-		entries, err := fs.ReadDir(fsys, runID)
+		safePath := safeResolve(baseDir, runID)
+
+		entries, err := fs.ReadDir(fsys, safePath)
 		if err != nil {
 			panic(err)
 		}
 
 		var list []NamedFileContainerResourceURL
@@ -202,25 +208,25 @@ func downloads(router *httprouter.Router, fsys fs.FS) {
 	})
 
 	router.GET("/download/:container", func(w http.ResponseWriter, req *http.Request, params httprouter.Params) {
 		container := params.ByName("container")
 		itemPath := req.URL.Query().Get("itemPath")
-		dirPath := fmt.Sprintf("%s/%s", container, itemPath)
+		safePath := safeResolve(baseDir, filepath.Join(container, itemPath))
 
 		var files []ContainerItem
-		err := fs.WalkDir(fsys, dirPath, func(path string, entry fs.DirEntry, err error) error {
+		err := fs.WalkDir(fsys, safePath, func(path string, entry fs.DirEntry, err error) error {
 			if !entry.IsDir() {
-				rel, err := filepath.Rel(dirPath, path)
+				rel, err := filepath.Rel(safePath, path)
 				if err != nil {
 					panic(err)
 				}
 
 				// if it was upload as gzip
 				rel = strings.TrimSuffix(rel, gzipExtension)
 
 				files = append(files, ContainerItem{
-					Path:            fmt.Sprintf("%s/%s", itemPath, rel),
+					Path:            filepath.Join(itemPath, rel),
 					ItemType:        "file",
 					ContentLocation: fmt.Sprintf("http://%s/artifact/%s/%s/%s", req.Host, container, itemPath, rel),
 				})
 			}
 			return nil
@@ -243,14 +249,16 @@ func downloads(router *httprouter.Router, fsys fs.FS) {
 	})
 
 	router.GET("/artifact/*path", func(w http.ResponseWriter, req *http.Request, params httprouter.Params) {
 		path := params.ByName("path")[1:]
 
-		file, err := fsys.Open(path)
+		safePath := safeResolve(baseDir, path)
+
+		file, err := fsys.Open(safePath)
 		if err != nil {
 			// try gzip file
-			file, err = fsys.Open(path + gzipExtension)
+			file, err = fsys.Open(safePath + gzipExtension)
 			if err != nil {
 				panic(err)
 			}
 			w.Header().Add("Content-Encoding", "gzip")
 		}
@@ -271,13 +279,13 @@ func Serve(ctx context.Context, artifactPath string, addr string, port string) c
 	}
 
 	router := httprouter.New()
 
 	logger.Debugf("Artifacts base path '%s'", artifactPath)
-	fs := os.DirFS(artifactPath)
-	uploads(router, MkdirFsImpl{artifactPath, fs})
-	downloads(router, fs)
+	fsys := readWriteFSImpl{}
+	uploads(router, artifactPath, fsys)
+	downloads(router, artifactPath, fsys)
 
 	server := &http.Server{
 		Addr:              fmt.Sprintf("%s:%s", addr, port),
 		ReadHeaderTimeout: 2 * time.Second,
 		Handler:           router,
diff --git a/pkg/artifacts/server_test.go b/pkg/artifacts/server_test.go
index 6f865dc..259c954 100644
--- a/pkg/artifacts/server_test.go
+++ b/pkg/artifacts/server_test.go
@@ -2,11 +2,10 @@ package artifacts
 
 import (
 	"context"
 	"encoding/json"
 	"fmt"
-	"io/fs"
 	"net/http"
 	"net/http/httptest"
 	"os"
 	"path"
 	"path/filepath"
@@ -19,57 +18,56 @@ import (
 	"github.com/nektos/act/pkg/runner"
 	log "github.com/sirupsen/logrus"
 	"github.com/stretchr/testify/assert"
 )
 
-type MapFsImpl struct {
-	fstest.MapFS
+type writableMapFile struct {
+	fstest.MapFile
 }
 
-func (fsys MapFsImpl) MkdirAll(path string, perm fs.FileMode) error {
-	// mocked no-op
-	return nil
+func (f *writableMapFile) Write(data []byte) (int, error) {
+	f.Data = data
+	return len(data), nil
 }
 
-type WritableFile struct {
-	fs.File
-	fsys fstest.MapFS
-	path string
+func (f *writableMapFile) Close() error {
+	return nil
 }
 
-func (file WritableFile) Write(data []byte) (int, error) {
-	file.fsys[file.path].Data = data
-	return len(data), nil
+type writeMapFS struct {
+	fstest.MapFS
 }
 
-func (fsys MapFsImpl) Open(path string) (fs.File, error) {
-	var file = fstest.MapFile{
-		Data: []byte("content2"),
+func (fsys writeMapFS) OpenWritable(name string) (WritableFile, error) {
+	var file = &writableMapFile{
+		MapFile: fstest.MapFile{
+			Data: []byte("content2"),
+		},
 	}
-	fsys.MapFS[path] = &file
+	fsys.MapFS[name] = &file.MapFile
 
-	result, err := fsys.MapFS.Open(path)
-	return WritableFile{result, fsys.MapFS, path}, err
+	return file, nil
 }
 
-func (fsys MapFsImpl) OpenAtEnd(path string) (fs.File, error) {
-	var file = fstest.MapFile{
-		Data: []byte("content2"),
+func (fsys writeMapFS) OpenAppendable(name string) (WritableFile, error) {
+	var file = &writableMapFile{
+		MapFile: fstest.MapFile{
+			Data: []byte("content2"),
+		},
 	}
-	fsys.MapFS[path] = &file
+	fsys.MapFS[name] = &file.MapFile
 
-	result, err := fsys.MapFS.Open(path)
-	return WritableFile{result, fsys.MapFS, path}, err
+	return file, nil
 }
 
 func TestNewArtifactUploadPrepare(t *testing.T) {
 	assert := assert.New(t)
 
 	var memfs = fstest.MapFS(map[string]*fstest.MapFile{})
 
 	router := httprouter.New()
-	uploads(router, MapFsImpl{memfs})
+	uploads(router, "artifact/server/path", writeMapFS{memfs})
 
 	req, _ := http.NewRequest("POST", "http://localhost/_apis/pipelines/workflows/1/artifacts", nil)
 	rr := httptest.NewRecorder()
 
 	router.ServeHTTP(rr, req)
@@ -91,11 +89,11 @@ func TestArtifactUploadBlob(t *testing.T) {
 	assert := assert.New(t)
 
 	var memfs = fstest.MapFS(map[string]*fstest.MapFile{})
 
 	router := httprouter.New()
-	uploads(router, MapFsImpl{memfs})
+	uploads(router, "artifact/server/path", writeMapFS{memfs})
 
 	req, _ := http.NewRequest("PUT", "http://localhost/upload/1?itemPath=some/file", strings.NewReader("content"))
 	rr := httptest.NewRecorder()
 
 	router.ServeHTTP(rr, req)
@@ -109,20 +107,20 @@ func TestArtifactUploadBlob(t *testing.T) {
 	if err != nil {
 		panic(err)
 	}
 
 	assert.Equal("success", response.Message)
-	assert.Equal("content", string(memfs["1/some/file"].Data))
+	assert.Equal("content", string(memfs["artifact/server/path/1/some/file"].Data))
 }
 
 func TestFinalizeArtifactUpload(t *testing.T) {
 	assert := assert.New(t)
 
 	var memfs = fstest.MapFS(map[string]*fstest.MapFile{})
 
 	router := httprouter.New()
-	uploads(router, MapFsImpl{memfs})
+	uploads(router, "artifact/server/path", writeMapFS{memfs})
 
 	req, _ := http.NewRequest("PATCH", "http://localhost/_apis/pipelines/workflows/1/artifacts", nil)
 	rr := httptest.NewRecorder()
 
 	router.ServeHTTP(rr, req)
@@ -142,17 +140,17 @@ func TestFinalizeArtifactUpload(t *testing.T) {
 
 func TestListArtifacts(t *testing.T) {
 	assert := assert.New(t)
 
 	var memfs = fstest.MapFS(map[string]*fstest.MapFile{
-		"1/file.txt": {
+		"artifact/server/path/1/file.txt": {
 			Data: []byte(""),
 		},
 	})
 
 	router := httprouter.New()
-	downloads(router, memfs)
+	downloads(router, "artifact/server/path", memfs)
 
 	req, _ := http.NewRequest("GET", "http://localhost/_apis/pipelines/workflows/1/artifacts", nil)
 	rr := httptest.NewRecorder()
 
 	router.ServeHTTP(rr, req)
@@ -174,17 +172,17 @@ func TestListArtifacts(t *testing.T) {
 
 func TestListArtifactContainer(t *testing.T) {
 	assert := assert.New(t)
 
 	var memfs = fstest.MapFS(map[string]*fstest.MapFile{
-		"1/some/file": {
+		"artifact/server/path/1/some/file": {
 			Data: []byte(""),
 		},
 	})
 
 	router := httprouter.New()
-	downloads(router, memfs)
+	downloads(router, "artifact/server/path", memfs)
 
 	req, _ := http.NewRequest("GET", "http://localhost/download/1?itemPath=some/file", nil)
 	rr := httptest.NewRecorder()
 
 	router.ServeHTTP(rr, req)
@@ -198,26 +196,26 @@ func TestListArtifactContainer(t *testing.T) {
 	if err != nil {
 		panic(err)
 	}
 
 	assert.Equal(1, len(response.Value))
-	assert.Equal("some/file/.", response.Value[0].Path)
+	assert.Equal("some/file", response.Value[0].Path)
 	assert.Equal("file", response.Value[0].ItemType)
 	assert.Equal("http://localhost/artifact/1/some/file/.", response.Value[0].ContentLocation)
 }
 
 func TestDownloadArtifactFile(t *testing.T) {
 	assert := assert.New(t)
 
 	var memfs = fstest.MapFS(map[string]*fstest.MapFile{
-		"1/some/file": {
+		"artifact/server/path/1/some/file": {
 			Data: []byte("content"),
 		},
 	})
 
 	router := httprouter.New()
-	downloads(router, memfs)
+	downloads(router, "artifact/server/path", memfs)
 
 	req, _ := http.NewRequest("GET", "http://localhost/artifact/1/some/file", nil)
 	rr := httptest.NewRecorder()
 
 	router.ServeHTTP(rr, req)
@@ -258,10 +256,11 @@ func TestArtifactFlow(t *testing.T) {
 		"ubuntu-latest": "node:16-buster-slim",
 	}
 
 	tables := []TestJobFileInfo{
 		{"testdata", "upload-and-download", "push", "", platforms, ""},
+		{"testdata", "GHSL-2023-004", "push", "", platforms, ""},
 	}
 	log.SetLevel(log.DebugLevel)
 
 	for _, table := range tables {
 		runTestJobFile(ctx, t, table)
@@ -308,5 +307,83 @@ func runTestJobFile(ctx context.Context, t *testing.T, tjfi TestJobFileInfo) {
 		}
 
 		fmt.Println("::endgroup::")
 	})
 }
+
+func TestMkdirFsImplSafeResolve(t *testing.T) {
+	assert := assert.New(t)
+
+	baseDir := "/foo/bar"
+
+	tests := map[string]struct {
+		input string
+		want  string
+	}{
+		"simple":         {input: "baz", want: "/foo/bar/baz"},
+		"nested":         {input: "baz/blue", want: "/foo/bar/baz/blue"},
+		"dots in middle": {input: "baz/../../blue", want: "/foo/bar/blue"},
+		"leading dots":   {input: "../../parent", want: "/foo/bar/parent"},
+		"root path":      {input: "/root", want: "/foo/bar/root"},
+		"root":           {input: "/", want: "/foo/bar"},
+		"empty":          {input: "", want: "/foo/bar"},
+	}
+
+	for name, tc := range tests {
+		t.Run(name, func(t *testing.T) {
+			assert.Equal(tc.want, safeResolve(baseDir, tc.input))
+		})
+	}
+}
+
+func TestDownloadArtifactFileUnsafePath(t *testing.T) {
+	assert := assert.New(t)
+
+	var memfs = fstest.MapFS(map[string]*fstest.MapFile{
+		"artifact/server/path/some/file": {
+			Data: []byte("content"),
+		},
+	})
+
+	router := httprouter.New()
+	downloads(router, "artifact/server/path", memfs)
+
+	req, _ := http.NewRequest("GET", "http://localhost/artifact/2/../../some/file", nil)
+	rr := httptest.NewRecorder()
+
+	router.ServeHTTP(rr, req)
+
+	if status := rr.Code; status != http.StatusOK {
+		assert.FailNow(fmt.Sprintf("Wrong status: %d", status))
+	}
+
+	data := rr.Body.Bytes()
+
+	assert.Equal("content", string(data))
+}
+
+func TestArtifactUploadBlobUnsafePath(t *testing.T) {
+	assert := assert.New(t)
+
+	var memfs = fstest.MapFS(map[string]*fstest.MapFile{})
+
+	router := httprouter.New()
+	uploads(router, "artifact/server/path", writeMapFS{memfs})
+
+	req, _ := http.NewRequest("PUT", "http://localhost/upload/1?itemPath=../../some/file", strings.NewReader("content"))
+	rr := httptest.NewRecorder()
+
+	router.ServeHTTP(rr, req)
+
+	if status := rr.Code; status != http.StatusOK {
+		assert.Fail("Wrong status")
+	}
+
+	response := ResponseMessage{}
+	err := json.Unmarshal(rr.Body.Bytes(), &response)
+	if err != nil {
+		panic(err)
+	}
+
+	assert.Equal("success", response.Message)
+	assert.Equal("content", string(memfs["artifact/server/path/1/some/file"].Data))
+}
diff --git a/pkg/artifacts/testdata/GHSL-2023-004/artifacts.yml b/pkg/artifacts/testdata/GHSL-2023-004/artifacts.yml
new file mode 100644
index 0000000..e717f14
--- /dev/null
+++ b/pkg/artifacts/testdata/GHSL-2023-004/artifacts.yml
@@ -0,0 +1,43 @@
+
+name: "GHSL-2023-0004"
+on: push
+
+jobs:
+  test-artifacts:
+    runs-on: ubuntu-latest
+    steps:
+      - run: echo "hello world" > test.txt
+      - name: curl upload
+        uses: wei/curl@v1
+        with:
+          args: -s --fail ${ACTIONS_RUNTIME_URL}upload/1?itemPath=../../my-artifact/secret.txt --upload-file test.txt
+      - uses: actions/download-artifact@v2
+        with:
+          name: my-artifact
+          path: test-artifacts
+      - name: 'Verify Artifact #1'
+        run: |
+          file="test-artifacts/secret.txt"
+          if [ ! -f $file ] ; then
+            echo "Expected file does not exist"
+            exit 1
+          fi
+          if [ "$(cat $file)" != "hello world" ] ; then
+            echo "File contents of downloaded artifact are incorrect"
+            exit 1
+          fi
+      - name: Verify download should work by clean extra dots
+        uses: wei/curl@v1
+        with:
+          args: --path-as-is -s -o out.txt --fail ${ACTIONS_RUNTIME_URL}artifact/1/../../../1/my-artifact/secret.txt
+      - name: 'Verify download content'
+        run: |
+          file="out.txt"
+          if [ ! -f $file ] ; then
+            echo "Expected file does not exist"
+            exit 1
+          fi
+          if [ "$(cat $file)" != "hello world" ] ; then
+            echo "File contents of downloaded artifact are incorrect"
+            exit 1
+          fi

Vulnerable code: 	"path"
type MkdirFS interface {
	fs.FS
	MkdirAll(path string, perm fs.FileMode) error
	Open(name string) (fs.File, error)
	OpenAtEnd(name string) (fs.File, error)
type MkdirFsImpl struct {
	dir string
	fs.FS
func (fsys MkdirFsImpl) MkdirAll(path string, perm fs.FileMode) error {
	return os.MkdirAll(fsys.dir+"/"+path, perm)
func (fsys MkdirFsImpl) Open(name string) (fs.File, error) {
	return os.OpenFile(fsys.dir+"/"+name, os.O_CREATE|os.O_RDWR|os.O_TRUNC, 0644)
func (fsys MkdirFsImpl) OpenAtEnd(name string) (fs.File, error) {
	file, err := os.OpenFile(fsys.dir+"/"+name, os.O_CREATE|os.O_RDWR, 0644)

func uploads(router *httprouter.Router, fsys MkdirFS) {
		filePath := fmt.Sprintf("%s/%s", runID, itemPath)
		err := fsys.MkdirAll(path.Dir(filePath), os.ModePerm)
		if err != nil {
			panic(err)
		}

		file, err := func() (fs.File, error) {
				return fsys.OpenAtEnd(filePath)
			return fsys.Open(filePath)
func downloads(router *httprouter.Router, fsys fs.FS) {
		entries, err := fs.ReadDir(fsys, runID)
		dirPath := fmt.Sprintf("%s/%s", container, itemPath)
		err := fs.WalkDir(fsys, dirPath, func(path string, entry fs.DirEntry, err error) error {
				rel, err := filepath.Rel(dirPath, path)
					Path:            fmt.Sprintf("%s/%s", itemPath, rel),
		file, err := fsys.Open(path)
			file, err = fsys.Open(path + gzipExtension)
	fs := os.DirFS(artifactPath)
	uploads(router, MkdirFsImpl{artifactPath, fs})
	downloads(router, fs)
	"io/fs"
type MapFsImpl struct {
	fstest.MapFS
func (fsys MapFsImpl) MkdirAll(path string, perm fs.FileMode) error {
	// mocked no-op
	return nil
type WritableFile struct {
	fs.File
	fsys fstest.MapFS
	path string
func (file WritableFile) Write(data []byte) (int, error) {
	file.fsys[file.path].Data = data
	return len(data), nil
func (fsys MapFsImpl) Open(path string) (fs.File, error) {
	var file = fstest.MapFile{
		Data: []byte("content2"),
	fsys.MapFS[path] = &file
	result, err := fsys.MapFS.Open(path)
	return WritableFile{result, fsys.MapFS, path}, err
func (fsys MapFsImpl) OpenAtEnd(path string) (fs.File, error) {
	var file = fstest.MapFile{
		Data: []byte("content2"),
	fsys.MapFS[path] = &file
	result, err := fsys.MapFS.Open(path)
	return WritableFile{result, fsys.MapFS, path}, err
	uploads(router, MapFsImpl{memfs})
	uploads(router, MapFsImpl{memfs})
	assert.Equal("content", string(memfs["1/some/file"].Data))
	uploads(router, MapFsImpl{memfs})
		"1/file.txt": {
	downloads(router, memfs)
		"1/some/file": {
	downloads(router, memfs)
	assert.Equal("some/file/.", response.Value[0].Path)
		"1/some/file": {
	downloads(router, memfs)
Fixed code:   "go.testTimeout": "300s",
type WritableFile interface {
	io.WriteCloser
type WriteFS interface {
	OpenWritable(name string) (WritableFile, error)
	OpenAppendable(name string) (WritableFile, error)
type readWriteFSImpl struct {
func (fwfs readWriteFSImpl) Open(name string) (fs.File, error) {
	return os.Open(name)
func (fwfs readWriteFSImpl) OpenWritable(name string) (WritableFile, error) {
	if err := os.MkdirAll(filepath.Dir(name), os.ModePerm); err != nil {
		return nil, err
	}
	return os.OpenFile(name, os.O_CREATE|os.O_RDWR|os.O_TRUNC, 0644)
}

func (fwfs readWriteFSImpl) OpenAppendable(name string) (WritableFile, error) {
	if err := os.MkdirAll(filepath.Dir(name), os.ModePerm); err != nil {
		return nil, err
	}
	file, err := os.OpenFile(name, os.O_CREATE|os.O_RDWR, 0644)
func safeResolve(baseDir string, relPath string) string {
	return filepath.Join(baseDir, filepath.Clean(filepath.Join(string(os.PathSeparator), relPath)))
}

func uploads(router *httprouter.Router, baseDir string, fsys WriteFS) {
		safeRunPath := safeResolve(baseDir, runID)
		safePath := safeResolve(safeRunPath, itemPath)
		file, err := func() (WritableFile, error) {
				return fsys.OpenAppendable(safePath)
			return fsys.OpenWritable(safePath)
func downloads(router *httprouter.Router, baseDir string, fsys fs.FS) {
		safePath := safeResolve(baseDir, runID)

		entries, err := fs.ReadDir(fsys, safePath)
		safePath := safeResolve(baseDir, filepath.Join(container, itemPath))
		err := fs.WalkDir(fsys, safePath, func(path string, entry fs.DirEntry, err error) error {
				rel, err := filepath.Rel(safePath, path)
					Path:            filepath.Join(itemPath, rel),
		safePath := safeResolve(baseDir, path)

		file, err := fsys.Open(safePath)
			file, err = fsys.Open(safePath + gzipExtension)
	fsys := readWriteFSImpl{}
	uploads(router, artifactPath, fsys)
	downloads(router, artifactPath, fsys)
type writableMapFile struct {
	fstest.MapFile
func (f *writableMapFile) Write(data []byte) (int, error) {
	f.Data = data
	return len(data), nil
func (f *writableMapFile) Close() error {
	return nil
type writeMapFS struct {
	fstest.MapFS
func (fsys writeMapFS) OpenWritable(name string) (WritableFile, error) {
	var file = &writableMapFile{
		MapFile: fstest.MapFile{
			Data: []byte("content2"),
		},
	fsys.MapFS[name] = &file.MapFile
	return file, nil
func (fsys writeMapFS) OpenAppendable(name string) (WritableFile, error) {
	var file = &writableMapFile{
		MapFile: fstest.MapFile{
			Data: []byte("content2"),
		},
	fsys.MapFS[name] = &file.MapFile
	return file, nil
	uploads(router, "artifact/server/path", writeMapFS{memfs})
	uploads(router, "artifact/server/path", writeMapFS{memfs})
	assert.Equal("content", string(memfs["artifact/server/path/1/some/file"].Data))
	uploads(router, "artifact/server/path", writeMapFS{memfs})
		"artifact/server/path/1/file.txt": {
	downloads(router, "artifact/server/path", memfs)
		"artifact/server/path/1/some/file": {
	downloads(router, "artifact/server/path", memfs)
	assert.Equal("some/file", response.Value[0].Path)
		"artifact/server/path/1/some/file": {
	downloads(router, "artifact/server/path", memfs)
		{"testdata", "GHSL-2023-004", "push", "", platforms, ""},

func TestMkdirFsImplSafeResolve(t *testing.T) {
	assert := assert.New(t)

	baseDir := "/foo/bar"

	tests := map[string]struct {
		input string
		want  string
	}{
		"simple":         {input: "baz", want: "/foo/bar/baz"},
		"nested":         {input: "baz/blue", want: "/foo/bar/baz/blue"},
		"dots in middle": {input: "baz/../../blue", want: "/foo/bar/blue"},
		"leading dots":   {input: "../../parent", want: "/foo/bar/parent"},
		"root path":      {input: "/root", want: "/foo/bar/root"},
		"root":           {input: "/", want: "/foo/bar"},
		"empty":          {input: "", want: "/foo/bar"},
	}

	for name, tc := range tests {
		t.Run(name, func(t *testing.T) {
			assert.Equal(tc.want, safeResolve(baseDir, tc.input))
		})
	}
}

func TestDownloadArtifactFileUnsafePath(t *testing.T) {
	assert := assert.New(t)

	var memfs = fstest.MapFS(map[string]*fstest.MapFile{
		"artifact/server/path/some/file": {
			Data: []byte("content"),
		},
	})

	router := httprouter.New()
	downloads(router, "artifact/server/path", memfs)

	req, _ := http.NewRequest("GET", "http://localhost/artifact/2/../../some/file", nil)
	rr := httptest.NewRecorder()

	router.ServeHTTP(rr, req)

	if status := rr.Code; status != http.StatusOK {
		assert.FailNow(fmt.Sprintf("Wrong status: %d", status))
	}

	data := rr.Body.Bytes()

	assert.Equal("content", string(data))
}

func TestArtifactUploadBlobUnsafePath(t *testing.T) {
	assert := assert.New(t)

	var memfs = fstest.MapFS(map[string]*fstest.MapFile{})

	router := httprouter.New()
	uploads(router, "artifact/server/path", writeMapFS{memfs})

	req, _ := http.NewRequest("PUT", "http://localhost/upload/1?itemPath=../../some/file", strings.NewReader("content"))
	rr := httptest.NewRecorder()

	router.ServeHTTP(rr, req)

	if status := rr.Code; status != http.StatusOK {
		assert.Fail("Wrong status")
	}

	response := ResponseMessage{}
	err := json.Unmarshal(rr.Body.Bytes(), &response)
	if err != nil {
		panic(err)
	}

	assert.Equal("success", response.Message)
	assert.Equal("content", string(memfs["artifact/server/path/1/some/file"].Data))
}

name: "GHSL-2023-0004"
on: push

jobs:
  test-artifacts:
    runs-on: ubuntu-latest
    steps:
      - run: echo "hello world" > test.txt
      - name: curl upload
        uses: wei/curl@v1
        with:
          args: -s --fail ${ACTIONS_RUNTIME_URL}upload/1?itemPath=../../my-artifact/secret.txt --upload-file test.txt
      - uses: actions/download-artifact@v2
        with:
          name: my-artifact
          path: test-artifacts
      - name: 'Verify Artifact #1'
        run: |
          file="test-artifacts/secret.txt"
          if [ ! -f $file ] ; then
            echo "Expected file does not exist"
            exit 1
          fi
          if [ "$(cat $file)" != "hello world" ] ; then
            echo "File contents of downloaded artifact are incorrect"
            exit 1
          fi
      - name: Verify download should work by clean extra dots
        uses: wei/curl@v1
        with:
          args: --path-as-is -s -o out.txt --fail ${ACTIONS_RUNTIME_URL}artifact/1/../../../1/my-artifact/secret.txt
      - name: 'Verify download content'
        run: |
          file="out.txt"
          if [ ! -f $file ] ; then
            echo "Expected file does not exist"
            exit 1
          fi
          if [ "$(cat $file)" != "hello world" ] ; then
            echo "File contents of downloaded artifact are incorrect"
            exit 1
          fi