golang/go

x/exp/cmd/txtar: expansion of environment variables and writing outside of the current directory seems risky

Open

#46,741 opened on Jun 14, 2021

View on GitHub
 (6 comments) (3 reactions) (0 assignees)Go (19,008 forks)batch import
NeedsFixhelp wanted

Repository metrics

Stars
 (133,883 stars)
PR merge metrics
 (No merged PRs in 30d)

Description

The x/exp/cmd/txtar command does not limit the scope of files it might overwrite, which seems risky (e.g., see https://snyk.io/research/zip-slip-vulnerability). Additionally, the expansion of environment variables could make it even easier for attackers to construct malicious files:

$ go install golang.org/x/exp/cmd/txtar@latest
$ cat hmm.txt
-- $HOME/.ssh/not_authorized_keys --
hmm
$ txtar -x < hmm.txt
$ cat ~/.ssh/not_authorized_keys
hmm

(Being plain text gives a small advantage that it's a bit harder to hide the malicious path, but for sufficiently large txtar files, a user is unlikely to review all of the file paths.)

I'd recommend that x/exp/cmd/txtar default to only writing to files within the current directory. If any resolve outside of the current directory, txtar should exit with an error. (I think exiting without writing any files would be safest, but it seems fine to still write in-scope files if that happens to be easier with txtar's implementation.) A flag can be provided to allow expansion to arbitrary file paths.

I'd also recommend not expanding environment variables by default, instead requiring a flag for that functionality.

/cc @bcmills @rsc @FiloSottile

Contributor guide