Default cache directory is vulnerable to symlink attacks

[aripollak]

The default temporary directory used by ResultCache is predictable and /tmp is world-writable by default, so using rubocop with caching on a system shared with other users is vulnerable to a symlink attack, as described here.

Because the cache needs to persist across program runs, I think the easiest option would be to default the cache directory to something like ~/.cache/rubocop or the current directory, since they should not be world-writable by default.

[jonas054]

We talked about using the home directory when we implemented the cache feature, but we chose to not "pollute" that directory with temporary files.

I find it difficult to see what a symlink attack could mean for RuboCop in terms of dangerous changes to files. RuboCop never writes to an existing file. It either reads from a cache file, named something like b67f67fa8e38ecf2503eff17dc2a44fd, or it creates it. So the worst thing that could happen is cache files being written to a place where they shouldn't be.

So I'm not sure that we need to do anything. Please explain, if you see something that I don't.

If we're going to do something, there are other ways. Since the cache files are personal -- they're stored under /tmp/<user id> -- we can have write permissions for the user only. Today I think the permissions are set based on umask.

[aripollak]

At best, there's a race condition between the checking of the temp file's existence and the writing of a new one. So if a malicious user can create a symlink for one of the cache names after the file existence is checked before write, the target file would be overwritten with the permissions of the running user. As far as I know this is pretty difficult to exploit and I don't think a separate user could influence the contents of the file except through social engineering, but is theoretically possible. I believe that opening the file with mode File::CREAT|File::EXCL|File::WRONLY will avoid this by not writing to the file if it already exists.

Worse, a malicious user can create a Marshal-formatted file in the appropriate location before the target user's rubocop cache directory is created, which will then get evaluated and can result in arbitrary code execution with the permissions of the executing user. The only surefire protection against this that I can think of would be to use JSON as a serialization format. Maybe there can also be a check for file/directory permissions before loading to make sure it's owned by the current user & is not writable by anyone else, but filesystem ACLs might make that annoying to check and I'm not sure if it covers all the cases.

Both of these problems would be moot if the cache weren't in a predictable directory name under /tmp, or if the cache isn't turned on by default and there's a warning against turning it on for a shared system. That would also mean that future caching algorithm changes wouldn't matter so much from a security perspective.

Both OS X and Linux distributions have relatively standard locations for exactly this type of thing so the home directory isn't polluted. For Linux, it's $XDG_CACHE_HOME or ~/.cache, as defined in http://standards.freedesktop.org/basedir-spec/basedir-spec-latest.html. For OS X, it seems to be ~/Library/Caches/.

Unrelatedly, if the default temp directory stays under /tmp, I'd also suggest naming the default directory something like rubocop_cache_UID, just to make it less likely to overlap with another program run by a different user that happens to just use a numerical directory name.