Issue2044

Lubomir Rintel 2013-04-03 11:29:50 EDT
Description of problem:

There are serveral problems with the way Jython creates class cache files, potentially leading to arbitrary code execution or information disclosure.

# (umask 000; jython -c 'import xmllib')
# ls -l '/usr/share/jython/Lib/xmllib$py.class'
-rw-rw-rw-. 1 root root 52874 Apr  3 17:24 /usr/share/jython/Lib/xmllib$py.class

Jython does not explicitly set permissions of the class files; therefore with weak umask it creates world-writable files, or discloses sensitive data that would be in a non-world-readable package file.

Also, the package writes to /usr/share, which it shouldn't; /var/cache would be more appropriate, but would still lead to a possibility of a content disclosure.

The only really portable and secure way to cache class files would be a directory in user's home with 0700 permissions.

It is currently even not possible to easily disable the caching, since the configuration file is not marked with %config and resides in /usr/share instead of /etc.

Version-Release number of selected component (if applicable):

jython-2.2.1-4.8.el6.x86_64

This was assigned CVE-2013-2027

This no longer appears to be the case with at least 2.7, possibly because we have better control of setting file permissions with JNR-Posix.

2.2 is not something we would fix.

I observe this still to be a problem on Linux. If, following the OP's example, I set umask 000 and provoke the creation of class files in Lib, they have permissions 0666.

I have a fix for this that will make private (rw to the owner only) compiled classes and the package cache, however permissive the umask. This does not affect files created by Python applications in general.

A user (or administrator) setting umask 0 is asking for trouble, and the system is only giving him what he asked for. However, he may have wanted permissive settings on files created by the application in a particular run, and be unaware of retained compiled files and the package cache then affecting later invocations. So I appreciate the argument in favour of restricting those files.

Observations on Linux, before the fix:

jeff@nahum:~/dev/jython-trunk$ umask 0
jeff@nahum:~/dev/jython-trunk$ ant
jeff@nahum:~/dev/jython-trunk$ ls -lG dist/Lib/as*.class
-rw-rw-rw- 1 jeff 26339 Jan 26 13:15 'dist/Lib/ast$py.class'
-rw-rw-rw- 1 jeff 18772 Jan 26 13:15 'dist/Lib/asynchat$py.class'
-rw-rw-rw- 1 jeff 37578 Jan 26 13:15 'dist/Lib/asyncore$py.class'
jeff@nahum:~/dev/jython-trunk$ dist/bin/jython -c "6*7"
jeff@nahum:~/dev/jython-trunk$ ls -lG dist/cachedir/packages/p*
-rw-rw-rw- 1 jeff 7592 Jan 26 13:19 dist/cachedir/packages/packages.idx
-rw-rw-rw- 1 jeff 4310 Jan 26 13:19 dist/cachedir/packages/postgresql-42.1.1.jre7.pkc
-rw-rw-rw- 1 jeff 1431 Jan 26 13:19 dist/cachedir/packages/profile.pkc

The class files are those created by compilation during the build. On the patched version:

jeff@nahum:~/jydev/Jython20200126$ umask 0
jeff@nahum:~/jydev/Jython20200126$ ant
jeff@nahum:~/jydev/Jython20200126$ ls -lG dist/Lib/as*.class
-rw------- 1 jeff 26339 Jan 26 13:16 'dist/Lib/ast$py.class'
-rw------- 1 jeff 18772 Jan 26 13:16 'dist/Lib/asynchat$py.class'
-rw------- 1 jeff 37578 Jan 26 13:16 'dist/Lib/asyncore$py.class'
jeff@nahum:~/jydev/Jython20200126$ dist/bin/jython -c "6*7"
jeff@nahum:~/jydev/Jython20200126$ ls -lG dist/cachedir/packages/p*
-rw------- 1 jeff 8008 Jan 26 13:24 dist/cachedir/packages/packages.idx
-rw------- 1 jeff 4314 Jan 26 13:24 dist/cachedir/packages/postgresql-42.1.1.jre7.pkc
-rw------- 1 jeff 1435 Jan 26 13:24 dist/cachedir/packages/profile.pkc

I'll give some thought to the location of the package cache as a separate step. Obviously no shared installation location should be writable by general users, so if it is not relocated to user storage, presumably it is ineffective?

Align title to CVE

Claiming fixed at https://hg.python.org/jython/rev/7e917a237b7a

I don't believe that https://hg.python.org/jython/rev/7e917a237b7a totally addresses the issue.

Line 847 of CachedJarsPackageManager.java has:

if (cachedir.isDirectory() || cachedir.mkdirs()) {

the cachedir.mkdirs() creates directories with rwxr-xr-x