alex / rply

An attempt to port David Beazley's PLY to RPython, and give it a cooler API.
BSD 3-Clause "New" or "Revised" License
381 stars 60 forks source link

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

Closed copyninja closed 9 years ago

copyninja commented 9 years ago

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 commented 9 years ago

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 commented 9 years ago

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

dstufft commented 9 years ago

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

alex commented 9 years ago

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

jwilk commented 9 years ago

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 commented 9 years ago

@jwilk fixed.