content-only allow/deny is insecure #83

Closed
timbertson opened this Issue Dec 17, 2013 · 2 comments

Comments

Projects
None yet
3 participants
@timbertson
Contributor

timbertson commented Dec 17, 2013

(firstly, this project is great - I was thinking about implementing almost exactly this the other day, glad to see it already exists).

I'm happy to see the allow/deny system, but I don't think it's sufficient. For example, if respectable-project/.envrc contained code to add $PWD/tools to the start of $PATH, I can copy respectable-project/.envrc into evil-project/.envrc, and it will be automatically executed by users who have authorized respectable-project/.envrc, even though it refers to a completely different tools/ directory (perhaps containing a malicious version of ls).

So perhaps the auth needs to contain the full path, as well as just the content.

@pwaller

This comment has been minimized.

Show comment
Hide comment
@pwaller

pwaller Dec 17, 2013

Member

I'm leaning towards "agree".

Member

pwaller commented Dec 17, 2013

I'm leaning towards "agree".

@zimbatm

This comment has been minimized.

Show comment
Hide comment
@zimbatm

zimbatm Dec 18, 2013

Member

Hi Tim, thanks for the kind words!

How likely is it that the attacker knows what .envrc you allowed and that
you load some it's untrusted git repo or archive, not notice the direnv
export notice and run one of the commands in the attacker's bin/ directory
? If yes do you think the attacker would have more chance by infecting the
./configure or Makefile ?

I don't think security is an absolute, we certainly increase the attack
surface in some ways and the more people adopt direnv the more lucrative a
target it will be. Similarly the attacker could publish a popular git repo,
wait for the people to allow the .envrc and add the program in the bin/
later.

That being said if a pull request was being submitted I would gladly accept
it ;) Thanks for keeping a vigilant eye out there.
On Dec 17, 2013 5:37 AM, "Tim Cuthbertson" notifications@github.com wrote:

(firstly, this project is great - I was thinking about implementing almost
exactly this the other day, glad to see it already exists).

I'm happy to see the allow/deny system, but I don't think it's sufficient.
For example, if respectable-project/.envrc contained code to add
$PWD/tools to the start of $PATH, I can copy respectable-project/.envrcinto
evil-project/.envrc, and it will be automatically executed by users who
have authorized respectable-project/.envrc, even though it refers to a
completely different tools/ directory (perhaps containing a malicious
version of ls).

So perhaps the auth needs to contain the full path, as well as just the
content.


Reply to this email directly or view it on GitHubhttps://github.com/zimbatm/direnv/issues/83
.

Member

zimbatm commented Dec 18, 2013

Hi Tim, thanks for the kind words!

How likely is it that the attacker knows what .envrc you allowed and that
you load some it's untrusted git repo or archive, not notice the direnv
export notice and run one of the commands in the attacker's bin/ directory
? If yes do you think the attacker would have more chance by infecting the
./configure or Makefile ?

I don't think security is an absolute, we certainly increase the attack
surface in some ways and the more people adopt direnv the more lucrative a
target it will be. Similarly the attacker could publish a popular git repo,
wait for the people to allow the .envrc and add the program in the bin/
later.

That being said if a pull request was being submitted I would gladly accept
it ;) Thanks for keeping a vigilant eye out there.
On Dec 17, 2013 5:37 AM, "Tim Cuthbertson" notifications@github.com wrote:

(firstly, this project is great - I was thinking about implementing almost
exactly this the other day, glad to see it already exists).

I'm happy to see the allow/deny system, but I don't think it's sufficient.
For example, if respectable-project/.envrc contained code to add
$PWD/tools to the start of $PATH, I can copy respectable-project/.envrcinto
evil-project/.envrc, and it will be automatically executed by users who
have authorized respectable-project/.envrc, even though it refers to a
completely different tools/ directory (perhaps containing a malicious
version of ls).

So perhaps the auth needs to contain the full path, as well as just the
content.


Reply to this email directly or view it on GitHubhttps://github.com/zimbatm/direnv/issues/83
.

timbertson added a commit to timbertson/direnv that referenced this issue Dec 26, 2013

rc.go: include path in fileHash
This makes two identical files at different locations
require separate authorization. Fixes #83

timbertson added a commit to timbertson/direnv that referenced this issue Dec 26, 2013

rc.go: include path in fileHash
This makes two identical files at different locations
require separate authorization. Fixes #83

@zimbatm zimbatm closed this in #88 Dec 27, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment