golang/go

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

Open

#46 741 ouverte le 14 juin 2021

Voir sur GitHub
 (6 commentaires) (3 réactions) (0 assignés)Go (19 008 forks)batch import
NeedsFixhelp wanted

Métriques du dépôt

Stars
 (133 883 stars)
Métriques de merge PR
 (Aucune PR mergée en 30 j)

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

Guide contributeur