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

CVE-2014-1938: still uses /tmp insecurely (forwarding from Debian BTS #737627) #42

Closed
copyninja opened this issue Sep 1, 2015 · 6 comments

Comments

@copyninja
Copy link

Hello,

There has been a security issue reported at Debian against rply. This issue is more than a year old. Can this be fixed by upstream?.

https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=737627

@jwilk
Copy link
Contributor

jwilk commented Sep 1, 2015

The simplest solution is of course to refuse the temptation for using /tmp as cache, because it's really not suitable for this purpose.
Instead, store the cache somewhere in $HOME.
I recommend following XDG Base Directory Specification, which says you should store cache files in $XDG_CACHE_HOME or $HOME/.cache.

The downside of this solution is that, unlike /tmp, the user's cache directory will be probably never cleaned, so obsolete cache files will lie there forever.
If you feel this is a significant problem, then the following method should work:

  1. Create temporary directory (using mkdtemp) and save the name in $HOME. Use this directory for storing cache files.
  2. If the cache directory name is already saved in $HOME, check with lstat() whether it's a real directory owned by us. If it's not, ditch it, and goto 1.

@alex
Copy link
Owner

alex commented Sep 1, 2015

Switching to $HOME seems reasonable. @dstufft does pip have a library it uses for doing cross platform cache dirs?

@dstufft
Copy link

dstufft commented Sep 1, 2015

We forked https://pypi.python.org/pypi/appdirs but I don't remember why we forked instead of bundled.

@alex
Copy link
Owner

alex commented Sep 1, 2015

@copyninja @jwilk: If one of you wants to confirm that #43 resolves the issue I can merge and do a release later today.

@jwilk
Copy link
Contributor

jwilk commented Sep 1, 2015

XDG Base Directory Specification says:

If, when attempting to write a file, the destination directory is non-existant an attempt should be made to create it with permission 0700.
If the destination directory exists already the permissions should not be changed.

But your code creates directories with default permissions.
Other than that, it looks good to me.

@alex
Copy link
Owner

alex commented Sep 1, 2015

@jwilk fixed.

@alex alex closed this as completed in 76d268a Sep 1, 2015
alex added a commit that referenced this issue Sep 1, 2015
Fixed #42 -- use a user cache directory
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants