GNU bug report logs - #41619
[PATCH] Mark python-shell-virtualenv-root as safe local variable

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 41619 in the body.
You can then email your comments to 41619 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: "Philip K." <philip.kaludercic <at> fau.de>
To: bug-gnu-emacs <at> gnu.org
Subject: [PATCH] Mark python-shell-virtualenv-root as safe local variable
Date: Sat, 30 May 2020 22:31:09 +0200

[Message part 1 (text/plain, inline)]

Hi,

I was trying to work with python today and it required me to use a
virtual environment. I noticed that python-mode already had some basic
virtualenv facilites, and it all worked. The only thing that annoyed me
was that I couldn't configure the setup to be properly persistent, since
python-shell-virtualenv-root couldn't be set as a directory local
variable.

The commentary section mentions

> ;; (setq python-shell-virtualenv-root "/path/to/env/")

but I couldn't find any other details on how the mode expects me to set
the variable.

Therefore I patched the few lines, and it works great.

[0001-Mark-python-shell-virtualenv-root-as-safe-for-direct.patch (text/x-patch, inline)]

From 31beb4931ae7800e719ac470ba8bd14a977f765c Mon Sep 17 00:00:00 2001
From: Philip K <philip <at> warpmail.net>
Date: Sat, 30 May 2020 21:47:51 +0200
Subject: [PATCH] Mark python-shell-virtualenv-root as safe for directories

---
 lisp/progmodes/python.el | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lisp/progmodes/python.el b/lisp/progmodes/python.el
index 1ca9f01963..f9537b4079 100644
--- a/lisp/progmodes/python.el
+++ b/lisp/progmodes/python.el
@@ -2090,7 +2090,8 @@ python-shell-virtualenv-root
 This variable, when set to a string, makes the environment to be
 modified such that shells are started within the specified
 virtualenv."
-  :type '(choice (const nil) string)
+  :type '(choice (const nil) directory)
+  :safe #'file-directory-p
   :group 'python)
 
 (defcustom python-shell-setup-codes nil
-- 
2.26.2

Message #10 received at 41619-done <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: "Philip K." <philip.kaludercic <at> fau.de>
Cc: 41619-done <at> debbugs.gnu.org
Subject: Re: bug#41619: [PATCH] Mark python-shell-virtualenv-root as safe local
 variable
Date: Sat, 13 Jun 2020 10:42:57 +0300

> From: "Philip K." <philip.kaludercic <at> fau.de>
> Date: Sat, 30 May 2020 22:31:09 +0200
> 
> I was trying to work with python today and it required me to use a
> virtual environment. I noticed that python-mode already had some basic
> virtualenv facilites, and it all worked. The only thing that annoyed me
> was that I couldn't configure the setup to be properly persistent, since
> python-shell-virtualenv-root couldn't be set as a directory local
> variable.
> 
> The commentary section mentions
> 
> > ;; (setq python-shell-virtualenv-root "/path/to/env/")
> 
> but I couldn't find any other details on how the mode expects me to set
> the variable.
> 
> Therefore I patched the few lines, and it works great.

Thanks, installed on the master branch.

In the future please accompany your changes with a ChangeLog-style
commit log entry; see CONTRIBUTE for the details.  (I added that for
you in this case.)

Message #13 received at 41619 <at> debbugs.gnu.org (full text, mbox):

From: Glenn Morris <rgm <at> gnu.org>
To: 41619 <at> debbugs.gnu.org
Cc: eliz <at> gnu.org, philip.kaludercic <at> fau.de
Subject: Re: bug#41619: [PATCH] Mark python-shell-virtualenv-root as safe
 local variable
Date: Sat, 13 Jun 2020 13:20:29 -0400

I don't understand how python-shell-virtualenv-root can be considered a
safe local variable. Surely it controls what "python" executable gets run.

As a test, I did:

python3 -m venv /tmp/foo

I then replaced /tmp/foo/bin/python with a shell-script:

 #!/bin/bash
 echo oh-oh

I then ran:
emacs -Q --eval '(setq python-shell-virtualenv-root "/tmp/foo")' -f python-mode
C-c C-p

This gives an inferior Python buffer with contents:

  oh-oh

  Process Python finished

In other words, this looks like a recipe for arbitrary code execution.

Message #16 received at 41619 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Glenn Morris <rgm <at> gnu.org>, "Philip K." <philip <at> warpmail.net>
Cc: 41619 <at> debbugs.gnu.org, philip.kaludercic <at> fau.de
Subject: Re: bug#41619: [PATCH] Mark python-shell-virtualenv-root as safe
 local variable
Date: Mon, 15 Jun 2020 21:53:50 +0300

> From: Glenn Morris <rgm <at> gnu.org>
> Date: Sat, 13 Jun 2020 13:20:29 -0400
> Cc: eliz <at> gnu.org, philip.kaludercic <at> fau.de
> 
> 
> I don't understand how python-shell-virtualenv-root can be considered a
> safe local variable. Surely it controls what "python" executable gets run.
> 
> As a test, I did:
> 
> python3 -m venv /tmp/foo
> 
> I then replaced /tmp/foo/bin/python with a shell-script:
> 
>  #!/bin/bash
>  echo oh-oh
> 
> I then ran:
> emacs -Q --eval '(setq python-shell-virtualenv-root "/tmp/foo")' -f python-mode
> C-c C-p
> 
> This gives an inferior Python buffer with contents:
> 
>   oh-oh
> 
>   Process Python finished
> 
> In other words, this looks like a recipe for arbitrary code execution.

Philip, could you please look into this?  TIA.

Message #19 received at 41619 <at> debbugs.gnu.org (full text, mbox):

From: "Philip K." <philip <at> warpmail.net>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 41619 <at> debbugs.gnu.org, rgm <at> gnu.org
Subject: Re: bug#41619: [PATCH] Mark python-shell-virtualenv-root as safe
 local variable
Date: Tue, 16 Jun 2020 18:52:07 +0200

Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: Glenn Morris <rgm <at> gnu.org>
>> Date: Sat, 13 Jun 2020 13:20:29 -0400
>> Cc: eliz <at> gnu.org, philip.kaludercic <at> fau.de
>> 
>> 
>> I don't understand how python-shell-virtualenv-root can be considered a
>> safe local variable. Surely it controls what "python" executable gets run.
>> 
>> As a test, I did:
>> 
>> python3 -m venv /tmp/foo
>> 
>> I then replaced /tmp/foo/bin/python with a shell-script:
>> 
>>  #!/bin/bash
>>  echo oh-oh
>> 
>> I then ran:
>> emacs -Q --eval '(setq python-shell-virtualenv-root "/tmp/foo")' -f python-mode
>> C-c C-p
>> 
>> This gives an inferior Python buffer with contents:
>> 
>>   oh-oh
>> 
>>   Process Python finished
>> 
>> In other words, this looks like a recipe for arbitrary code execution.
>
> Philip, could you please look into this?  TIA.

First of all, sorry for the delayed response. I look a look at how
python.el uses python-shell-virtualenv-root and how virtualenv works in
general, and I think that Glenn's analysis is corret. One would have to
make sure that /tmp/foo/bin/python is an actual Python installation. Now
on my system it seems like python/python3 is always a symbolic link to
/usr/bin/python3 (if the virtual enviornment was created using python3),
but checking is neither portable or totally solves the issue.
with all odds agains you, you cannot 

I've also noticed that in other places, people automatically activate
python environments, if for example in a shell, the path
`./venv/bin/activate` is valid, but nowhere could I find any validation
to ensure that arbitrary code isn't executed :(

Ultimatly, my estimation was wrong, and the variable shouldn't be marked
as safe, at least not with any heuristics that could warn the user if
the path is suspicious.

-- 
	Philip K.

Message #22 received at 41619 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: "Philip K." <philip <at> warpmail.net>
Cc: 41619 <at> debbugs.gnu.org, rgm <at> gnu.org
Subject: Re: bug#41619: [PATCH] Mark python-shell-virtualenv-root as safe
 local variable
Date: Tue, 16 Jun 2020 20:18:18 +0300

> From: "Philip K." <philip <at> warpmail.net>
> Cc: rgm <at> gnu.org, 41619 <at> debbugs.gnu.org
> Date: Tue, 16 Jun 2020 18:52:07 +0200
> 
> Ultimatly, my estimation was wrong, and the variable shouldn't be marked
> as safe, at least not with any heuristics that could warn the user if
> the path is suspicious.

So all we need is to remove the :safe attribute from the variable?  Or
something else?

Message #25 received at 41619 <at> debbugs.gnu.org (full text, mbox):

From: "Philip K." <philip <at> warpmail.net>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 41619 <at> debbugs.gnu.org, rgm <at> gnu.org
Subject: Re: bug#41619: [PATCH] Mark python-shell-virtualenv-root as safe
 local variable
Date: Tue, 16 Jun 2020 19:32:52 +0200

Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: "Philip K." <philip <at> warpmail.net>
>> Cc: rgm <at> gnu.org, 41619 <at> debbugs.gnu.org
>> Date: Tue, 16 Jun 2020 18:52:07 +0200
>> 
>> Ultimatly, my estimation was wrong, and the variable shouldn't be marked
>> as safe, at least not with any heuristics that could warn the user if
>> the path is suspicious.
>
> So all we need is to remove the :safe attribute from the variable?  Or
> something else?

That would make it harder for projects to hide malicious values of
python-shell-virtualenv-root, but it's still an attack vector in
principle.

-- 
	Philip K.

Message #28 received at 41619 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: "Philip K." <philip <at> warpmail.net>
Cc: 41619 <at> debbugs.gnu.org, rgm <at> gnu.org
Subject: Re: bug#41619: [PATCH] Mark python-shell-virtualenv-root as safe
 local variable
Date: Tue, 16 Jun 2020 20:54:06 +0300

> From: "Philip K." <philip <at> warpmail.net>
> Cc: rgm <at> gnu.org, 41619 <at> debbugs.gnu.org
> Date: Tue, 16 Jun 2020 19:32:52 +0200
> 
> > So all we need is to remove the :safe attribute from the variable?  Or
> > something else?
> 
> That would make it harder for projects to hide malicious values of
> python-shell-virtualenv-root, but it's still an attack vector in
> principle.

Then I don't think I understand how you suggest to fix this.

Message #31 received at 41619 <at> debbugs.gnu.org (full text, mbox):

From: "Philip K." <philip <at> warpmail.net>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 41619 <at> debbugs.gnu.org, rgm <at> gnu.org
Subject: Re: bug#41619: [PATCH] Mark python-shell-virtualenv-root as safe
 local variable
Date: Tue, 16 Jun 2020 21:49:44 +0200

Eli Zaretskii <eliz <at> gnu.org> writes:

>> That would make it harder for projects to hide malicious values of
>> python-shell-virtualenv-root, but it's still an attack vector in
>> principle.
>
> Then I don't think I understand how you suggest to fix this.

I don't know either, any directory with a properly configured
dir-locals.el file and a bin/python executable can be exploited if the
user doesn't pay attention in python-mode. 

As mentioned above, I agree that the best thing would be to unmark the
variable as safe. I'll try to find out more on how to avoid abitrary
code execution in python, and if there's some way, I would try to
implement it so that the variable can be marked as safe again.

-- 
	Philip K.

Message #34 received at 41619 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: "Philip K." <philip <at> warpmail.net>
Cc: 41619 <at> debbugs.gnu.org, rgm <at> gnu.org
Subject: Re: bug#41619: [PATCH] Mark python-shell-virtualenv-root as safe
 local variable
Date: Sat, 20 Jun 2020 11:47:44 +0300

> From: "Philip K." <philip <at> warpmail.net>
> Cc: rgm <at> gnu.org, 41619 <at> debbugs.gnu.org
> Date: Tue, 16 Jun 2020 21:49:44 +0200
> 
> As mentioned above, I agree that the best thing would be to unmark the
> variable as safe.

Done.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.