Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Fix for CVE-2014-0012 #296

Closed
wants to merge 1 commit into from

2 participants

@thoger

Commit acb672b attempted to fix insecure temporary file use in FileSystemBytecodeCache by introducing per-user temporary directory. However, if directory already exists, it is used without checking its ownership and permissions. Local attacker able to create temporary directory with victim's uid before victim does so can achieve the same impact as with the original issue.

This attempts to address the issue by introducing ownership, permission and type check and makes jinja2 raise an exception if existing directory has unexpected attributes.

Further discussion can be found in:
https://bugzilla.redhat.com/show_bug.cgi?id=1051421

@thoger thoger Fix CVE-2014-0012
Add checks for the per-user temporary directory.  If it already exists, make
sure that it:
- is owned by the current user
- is directory
- has expected permissions

This commit also fixes:
- nt -> n typo pointed out in the review of acb672b
- replace 448 with stat.S_IRWXU when setting directory mode
4fe07b7
@mitsuhiko
Owner

Fixed.

@mitsuhiko mitsuhiko closed this
@mitsuhiko
Owner

Sorry for the lack of feedback on this. I completely lost track of this issue and I did not subscribe to the red hat ticket so i failed to see the continued discussion.

@mitsuhiko
Owner

2.7.3 has just this fix in now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Feb 9, 2014
  1. @thoger

    Fix CVE-2014-0012

    thoger authored
    Add checks for the per-user temporary directory.  If it already exists, make
    sure that it:
    - is owned by the current user
    - is directory
    - has expected permissions
    
    This commit also fixes:
    - nt -> n typo pointed out in the review of acb672b
    - replace 448 with stat.S_IRWXU when setting directory mode
This page is out of date. Refresh to see the latest.
Showing with 10 additions and 3 deletions.
  1. +10 −3 jinja2/bccache.py
View
13 jinja2/bccache.py
@@ -16,6 +16,7 @@
"""
from os import path, listdir
import os
+import stat
import sys
import errno
import marshal
@@ -215,7 +216,7 @@ def _get_default_cache_dir(self):
# On windows the temporary directory is used specific unless
# explicitly forced otherwise. We can just use that.
- if os.name == 'n':
+ if os.name == 'nt':
return tmpdir
if not hasattr(os, 'getuid'):
raise RuntimeError('Cannot determine safe temp directory. You '
@@ -224,12 +225,18 @@ def _get_default_cache_dir(self):
dirname = '_jinja2-cache-%d' % os.getuid()
actual_dir = os.path.join(tmpdir, dirname)
try:
- # 448 == 0700
- os.mkdir(actual_dir, 448)
+ os.mkdir(actual_dir, stat.S_IRWXU) # 0o700
except OSError as e:
if e.errno != errno.EEXIST:
raise
+ actual_dir_stat = os.lstat(actual_dir)
+ if actual_dir_stat.st_uid != os.getuid() \
+ or not stat.S_ISDIR(actual_dir_stat.st_mode) \
+ or stat.S_IMODE(actual_dir_stat.st_mode) != stat.S_IRWXU:
+ raise RuntimeError('Temporary directory \'%s\' has an incorrect '
+ 'owner, permissions, or type.' % actual_dir)
+
return actual_dir
def _get_cache_filename(self, bucket):
Something went wrong with that request. Please try again.