Because tkinter.py uses exec to execute code from a file, it has a security bug. It searches for the file in the home dir. Apparently, on my system (don't know if its the same on others), the $HOME variable is the same as the non-root one when running Python with root priveleges
Steps to reproduce:
create a file called .Tk.py in your home folder
Whatever code is in that file will be executed.

run these three lines of code in Python:

import tkinter
w = tkinter.Tk()
w.mainloop()

I will add more details in later comments.

s/tkinter.py/tkinter.__init__

In Lib/tkinter/__init__.py:1801 , the readprofile function executes untrusted code.

specifically, when running Python using sudo (not in a root shell), the $HOME variable is preserved.

I made many mistakes in the original bug report. Here is a fixed one:

Because Lib/tkinter/__init__.py:1801 uses exec to execute code from a file, it has a security bug. It searches for the file in the home dir. Apparently, on my system, the $HOME variable is the same as the non-root one when running Python with root privileges using sudo.

Steps to reproduce:
create a file called .Tk.py in your home folder
Whatever code is in that file will be executed.

run these three lines of code in Python using sudo:

import tkinter
w = tkinter.Tk()

And the code in the .Tk.py will be executed (unless if you change the baseName for the Tk object)
There may be similar ways of running Python with root privileges preserving the environment variables in other OS'es
Using Kubuntu Linux (variant of Ubuntu Linux) 12.04

It is a well known fact that the readprofile function uses exec, and it has been like that for more than 18 years. The parameters baseName and className defines the execution of the files $HOME/.{className}.tcl, $HOME/.{className}.py, $HOME/.{baseName}.tcl, and $HOME/.{baseName}.py. The function's docstring actually say that.

Said that, I never needed to load custom code during the creation of a Tk instance. To me the existence of readprofile is unneeded.

But what is your proposal to the issue ?

On 17 October 2012 23:46, Guilherme Polo <report@bugs.python.org> wrote:

>
> Guilherme Polo added the comment:
>
> It is a well known fact that the readprofile function uses exec, and it
> has been like that for more than 18 years. The parameters baseName and
> className defines the execution of the files $HOME/.{className}.tcl,
> $HOME/.{className}.py, $HOME/.{baseName}.tcl, and $HOME/.{baseName}.py. The
> function's docstring actually say that.
>
> Said that, I never needed to load custom code during the creation of a Tk
> instance. To me the existence of readprofile is unneeded.
>
> But what is your proposal to the issue ?
>
> ----------
>
> _______________________________________
> Python tracker <report@bugs.python.org>
> <http://bugs.python.org/issue16248>
> _______________________________________
>

Simply remove the readprofile code (it is not documented and I never had to
use it)

Apparently when replying by email my old account name is shown ("mani and ram") Just so you know that "mani and ram" is me.

For something that has been around for so long, I would be a little more careful.

My suggestion is: no longer declare readprofile as an internal function; remove the direct call to readprofile from Tk.__init__; tell users about the new behavior, and the need to call readprofile themselves if they wish to.

In some years we might be able to estimate if readprofile is used at all, and then decide about removing it. Since I haven't been active as a Python committer, I will let the final decision to someone else.

It is possible with this bug to make a sudo IDLE edit a root-file.

oops ignore last msg

I can ignore it, but let us be honest. If you got sudo privilege already, why are you bothering to break (or whatever else) the system using IDLE ? The issue here did not give you the sudo privilege. If it did, then we have an actual security bug.

I think this is a legitimate security bug.
the malicious program needs to create a file with a certain name in the home dir.
If a user runs say IDLE (or another tk app) with root priveleges using sudo, the file will be run with root priveleges.

Ramchandra:  can you give an example of a realistic situation where the existence of this code in tkinter allows users to execute code *that they wouldn't be able to execute otherwise*?

@Mark Dickinson
Run the attached file, exploit.py, with normal priveleges and then run IDLE with sudo (something I did to actually uncover this bug!).
Then the file "/root/exploited" should contain "Exploit succeeded!"

Okay, but if a user can run IDLE with sudo, they presumably *already* have many other ways to use sudo to create files in /root, without using IDLE or tkinter.  That's why I said:  "*that they wouldn't be able to execute otherwise*".  I don't see the security issue here.

If I understand correctly, I think what Ramchandra is getting at is that if an attacker could manage to get a .Tk.py file into a user's home directory somehow, then the next time that user happens to do 'sudo idle', the attacker's code is executed with root privileges.

That said, I don't know that it would be any easier for an attacker to get such a file into such a place than to just do their maliciousness some other way.

I think Guilherme's suggestion of just making those who need it call it themselves, instead of at every tkinter startup, sounds good.

As Zachary and Ramchandra explained, the security issue is obvious: a non-sudoer user A can make a sudoer user B execute arbitrary code, simply by placing a file where IDLE will be run from.

This is the same reason Python has -s and -E options. The least we could do would be to disable readprofile() when sys.flags.ignore_environment is true.

And then user A is relying on user B executing IDLE via sudo?  Is that a normal thing to do?

> And then user A is relying on user B executing IDLE via sudo?  Is that
> a normal thing to do?

Well, I suppose that could be any Tk app, not just IDLE.
And I also suppose you could use IDLE to edit some file that is only
root-writable.

So if this is a security issue, should Python 2.6 also be fixed?

> So if this is a security issue, should Python 2.6 also be fixed?

Probably, if it's deemed important enough by our security RMs.

Isn't IDLE supposed to be a Python shell? As I understand this issue,
you'd have the same "exploit" by adding this to your .bashrc:

echo "EXPLOIT" > /root/exploit


Then, as a normal user, run:

sudo bash



It would be nice to get rid of the exec, but why is this an exploit?

As I understand it, this is not specifically about IDLE. Any Tk app would be vulnerable.

It is not IDLE specific. But I still fail to see how this actually is a
security bug. It doesn't give more power to the user than the user already
gave to it. If you are recklessly installing untrusted libraries or
anything for the matter, then you already have a lot of other problems.

Anyways, I would still go with my earlier option because I never used this
piece of code.

2012/11/1 Antoine Pitrou <report@bugs.python.org>

>
> Antoine Pitrou added the comment:
>
> As I understand it, this is not specifically about IDLE. Any Tk app would
> be vulnerable.
>
> ----------
>
> _______________________________________
> Python tracker <report@bugs.python.org>
> <http://bugs.python.org/issue16248>
> _______________________________________
>

I also don't find the scenario where an attacker has write privileges
to a user's home directory so disturbing -- there are juicier targets
(like .bashrc).


This constructed example using /tmp is a little more troubling:

$ cd /tmp
$ echo 'print("exploit")' > .Tk.py
$ export XAUTHORITY=$HOME/.Xauthority
$ unset HOME
$ python3.3

>>>import tkinter
>>> w = tkinter.Tk()
/usr/local/lib/python3.3/tkinter/__init__.py:1817: ResourceWarning: unclosed file <_io.TextIOWrapper name='./.Tk.py' mode='r' encoding='ANSI_X3.4-1968'>
  exec(open(class_py).read(), dir)
exploit

On 2 November 2012 01:48, Stefan Krah <report@bugs.python.org> wrote:

>
> Stefan Krah added the comment:
>
> Isn't IDLE supposed to be a Python shell? As I understand this issue,
> you'd have the same "exploit" by adding this to your .bashrc:
>
> echo "EXPLOIT" > /root/exploit
>
>
> Then, as a normal user, run:
>
> sudo bash
>
>
>
> It would be nice to get rid of the exec, but why is this an exploit?
>
> ----------
> nosy: +skrah
>
> _______________________________________
> Python tracker <report@bugs.python.org>
> <http://bugs.python.org/issue16248>
> _______________________________________
>

Almost nobody knows that when using tkinter, code in .Tk.py is executed.
(readprofile is not even documented!)
While in your example, it is quite easy to see that it will run .bashrc

Ramchandra Apte <report@bugs.python.org> wrote:
> Almost nobody knows that when using tkinter, code in .Tk.py is executed.
> (readprofile is not even documented!)
> While in your example, it is quite easy to see that it will run .bashrc

The point of the example is that it's "game over" anyway once an attacker
has write privileges to a user's home directory.

"sudo bash" is certainly a more common operation than "sudo tkapp.py",
and users are not in the habit of auditing .bashrc each time they launch
a shell.

In fact, I'd probably be more likely to notice a new file ".Tk.py" than
a small modification to my .bashrc.

That said, I absolutely agree that *ideally* tkinter apps should not
execute code from a startup file, especially if the startup file is
*not* in the user's home directory.

I'm gonna point people to the discussion about the "-s" flag of the Python interpreter (added as part of PEP 370), since the issue is conceptually identical:
http://mail.python.org/pipermail/python-dev/2008-January/076130.html

Adding Christian to the discussion since he was the author and implementor of that PEP.

It's gonna take a while to read this ticket ...

Some comments:
The code in site.py already does some checks, for example getuid() == geteuid(). System code and code that is run with administrator privileges shall be run with -Es to prevent code injection. See https://bugs.launchpad.net/ubuntu/+source/lsb/+bug/938869 comment #24 for an issue.

I'm all with Antoine's suggestion. readprofile() should not be executed when sys.flags.ignore_environment is set.

Here are a pair of trivial patches that implement Antoine's suggestion of not executing readprofile() if the -E flag is set.  Current tests seem to pass, but there are no new tests included because frankly I'm not sure of either how to test it or where.  Also, I can't seem to find any tests of the -E flag at all; does this change need a test?  No doc change either since the function isn't documented.

The patches apply equally well on 2.6 or 2.7, or 3.1 or 3.2, depending on whether this is determined to be security issue enough to mess with 2.6 and 3.1.

Thanks,

Zach

New changeset d8300842a0e9 by Antoine Pitrou in branch '3.2':
Issue #16248: Disable code execution from the user's home directory by tkinter when the -E flag is passed to Python.
http://hg.python.org/cpython/rev/d8300842a0e9

New changeset 10d04bdb05ab by Antoine Pitrou in branch '3.3':
Issue #16248: Disable code execution from the user's home directory by tkinter when the -E flag is passed to Python.
http://hg.python.org/cpython/rev/10d04bdb05ab

New changeset a4fc52da295b by Antoine Pitrou in branch 'default':
Issue #16248: Disable code execution from the user's home directory by tkinter when the -E flag is passed to Python.
http://hg.python.org/cpython/rev/a4fc52da295b

New changeset 822b472eff13 by Antoine Pitrou in branch '2.7':
Issue #16248: Disable code execution from the user's home directory by tkinter when the -E flag is passed to Python.
http://hg.python.org/cpython/rev/822b472eff13

Fixed. I will let Benjamin and Barry decide whether this deserves backporting to security branches. Benjamin, Barry, please do your job :)

New changeset 03b3124e9ea3 by Antoine Pitrou in branch '3.1':
Issue #16248: Disable code execution from the user's home directory by tkinter when the -E flag is passed to Python.
http://hg.python.org/cpython/rev/03b3124e9ea3

I suppose this should be closed.

The bug hasn't been closed deliberately. We need to announce the security fix and possibly acquire a CVE, too.

I believe we're also waiting on input from Barry about whether to apply the
patch to 2.6.

Does the 2.x patch apply cleanly to 2.6?  If so, then I think it should be applied (though I'd like to review it first).  2.6 is still under security maintenance until October 2013.  I'm thinking we'll probably do one last security release around that time.

> Does the 2.x patch apply cleanly to 2.6?

It should, if I remember correctly, though I haven't checked since
uploading it. I believe there were actually very few or no changes to the
file the patch is for between 2.6 and 2.7.

Release blocking for 2.6.9 (oh how I wish we could release block for specific Python versions).

> Barry A. Warsaw added the comment:
> 
> Does the 2.x patch apply cleanly to 2.6?

Perhaps it's your job as a release manager to check that ;-P

I'm working on applying the 2.x patch to 2.6, but one thing interesting of note: sudo, at least on Debian and derivatives going back at least to Squeeze, generally reset the environment by default (i.e. env_reset).  So you'd have to either have disabled env_reset in sudoers or use `sudo -E` the exploit.py.

> I'm working on applying the 2.x patch to 2.6, but one thing
> interesting of note: sudo, at least on Debian and derivatives going
> back at least to Squeeze, generally reset the environment by default
> (i.e. env_reset).  So you'd have to either have disabled env_reset in
> sudoers or use `sudo -E` the exploit.py.

Or you just have to use something else than Debian.

New changeset 936621d33c38 by Barry Warsaw in branch '2.6':
- Issue #16248: Disable code execution from the user's home directory by
http://hg.python.org/cpython/rev/936621d33c38

I think this has now been applied to all of 2.6, 2.7, 3.1, 3.2, 3.3, and 3.4.  So, closing.

The fix in 2.6, 2.7, 3.1 and 3.2 branches introduced UnboundLocalError occurring when a non-None baseName parameter is passed.
At least a part of fa82071bb7e1 should be backported to 2.6, 2.7, 3.1 and 3.2 branches.

$ python2.7 -c 'import Tkinter; print(repr(Tkinter.Tk(baseName="some_name")))'
Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "/usr/lib64/python2.7/lib-tk/Tkinter.py", line 1748, in __init__
    if not sys.flags.ignore_environment:
UnboundLocalError: local variable 'sys' referenced before assignment
$ python3.2 -c 'import tkinter; print(repr(tkinter.Tk(baseName="some_name")))'
Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "/usr/lib64/python3.2/tkinter/__init__.py", line 1734, in __init__
    if not sys.flags.ignore_environment:
UnboundLocalError: local variable 'sys' referenced before assignment

You're talking about a new bug.  Please open a new issue.

I'm closing this issue right now because I want to cut a Python 3.4 alpha release tomorrow, and this bug is marked Python 3.4 and "release blocker".

UnboundLocalError is effect of partially incorrect fix for this bug.
It is better to update Versions field.

New changeset 0f17aed78168 by Antoine Pitrou in branch '2.7':
Fix tkinter regression introduced by the security fix in #16248.
http://hg.python.org/cpython/rev/0f17aed78168

I've committed a fix to 2.7 (I hope it's really a fix, since I don't know how to test it).
I'll let Benjamin and Barry decide whether to backport to 2.6 and 3.2.
As for 3.1, it's pretty much dead.

Antoine Pitrou wrote:
> I've committed a fix to 2.7 (I hope it's really a fix, since I don't know how to test it).
> I'll let Benjamin and Barry decide whether to backport to 2.6 and 3.2.
> As for 3.1, it's pretty much dead.

That fix does work, but it should probably get a NEWS entry since it fixes a regression from 2.7.3 to 2.7.5.

Also, I think the same fix should be backported to all three of 2.6, 3.1, and 3.2.  The same regression as in 2.7.5 exists in 3.2.5, and would be introduced by the next (last?) releases of 2.6 and 3.1.

Sorry to have broken every possible version of Python :S

New changeset 00bcf202cc3f by Antoine Pitrou in branch '2.7':
Add NEWS entry for 0f17aed78168 (issue #16248)
http://hg.python.org/cpython/rev/00bcf202cc3f

> That fix does work, but it should probably get a NEWS entry since it
> fixes a regression from 2.7.3 to 2.7.5.

Done, thank you?

> Also, I think the same fix should be backported to all three of 2.6,
> 3.1, and 3.2.

Benjamin and Barry will have to decide what to do for 2.6 and 3.2.

3.2 is owned by Georg.

As for 3.1, feel free to apply. There will likely be a source release of it at some point.

New changeset 84f40562669f by Barry Warsaw in branch '2.6':
Fix UnboundLocalError regression due to previous incorrect fix for
http://hg.python.org/cpython/rev/84f40562669f

Applied the fix to 2.6 and null merged into 2.7.  This didn't need a NEWS entry because the regression hadn't been released yet.  I'll remove 2.6 from the Versions now.

Should be fixed now in 3.2 too.

New changeset c18c18774e24 by Georg Brandl in branch '3.2':
Fix tkinter regression introduced by the security fix in #16248.
http://hg.python.org/cpython/rev/c18c18774e24

Fix still needs to be applied on 3.1 branch.

New changeset c39f42f46a05 by Georg Brandl in branch '3.1':
Fix tkinter regression introduced by the security fix in #16248.
http://hg.python.org/cpython/rev/c39f42f46a05

Fix backported to 3.1, closing.