Fix for CVE-2014-0012 (Insecure temp folder creation)

[kilink]

I think this one slipped through the cracks somehow, but the fix for CVE-2014-1402 introduced another security issue, which was assigned CVE-2014-0012.

This PR simply delegates temporary directory creation to tempfile.mkdtemp() in FileSystemBytecodeCache, which does the right thing.

The tests pass, but the only question I have is in regard to the intended behavior of the FilesystemBytecodeCache. Should multiple instances of a FileSystemBytecodeCache initialized without the directory argument point to the same directory? If so, I would need to modify my PR to stash the directory as a class variable (or something) to work around the issue.

Please see the following links for discussion of the issue:

http://seclists.org/oss-sec/2014/q1/73

http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=734956

Note that it looks like this was fixed via a similar patch in Debian, but it would be nice to have this upstream.

[thoger]

Similar patch is used in Debian packages too:

http://patch-tracker.debian.org/patch/series/view/jinja2/2.7.2-2/fix_CVE-2014-0012.patch

Should multiple instances of a FileSystemBytecodeCache initialized without the directory argument point to the same directory?

Jinja2 documentation says:

Bytecode caches make it possible to store the generated bytecode on the file system or a different location to avoid parsing the templates on first use.

This is especially useful if you have a web application that is initialized on the first request and Jinja compiles many templates at once which slows down the application.

It seems re-use of cache across invocation is expected, which makes tempfile.mkdtemp() fix not usable. I also assume that approach has more issues with cache file clean up because of lack of temporary directory removal.

[kilink]

As I said, the directory name could be cached on the class to preserve it across instances of FileSystemBytecodeCache objects. I can change the PR, but first I'd like to get feedback from a Jinja2 committer.

The existing implementation doesn't clean up the directory it creates either, so I think that is a separate issue.

[thoger]

As I said, the directory name could be cached on the class to preserve it across instances of FileSystemBytecodeCache objects.

Is that something that should only work for multiple instances created during the one invocation of an application, or something that should work across application invocations? If I understand your brief hint correctly, that approach would not try to cover multiple invocations.

The existing implementation doesn't clean up the directory it creates either, so I think that is a separate issue.

The current implementation doesn't clean up, but there is a fixed limit on the number of cache files created - the number of templates used. With real temporary directory, it's that number multiplied by number of invocations.