Path traversal vulnerability in case of symlinks

[giampaolo]

From billiej...@gmail.com on November 26, 2007 21:58:50

Most ftp filesystem commands are dangerously affected by path traversal in
case the filesystem path passed as command argument is a symbolic link
pointing to a path outside the user's home directory.
Let's imagine the following scenario:

1. On a Unix system we got a user "foo" which has "/home/foo" as its home
directory.

2. We create a symbolic link pointing to a path outside its home directory,
e.g.: 

foo@uds:/home/foo$ pwd
/home/foo
foo@uds:/home/foo$ ln -s /tmp/baz link
foo@uds:/home/foo$ ls -l link
drwxrwxrwx 1 foo   user        1 2007-11-04 10:31 link -> /tmp/baz

3. We run pyftpdlib by creating a username called "foo" having "/home/foo"
as home directory, then we use an ftp client for connecting to the ftp server.

4. If /tmp/baz is a directory we can change the current directory by
issuing the "CWD link" command. If we then issue a LIST command the files
of /tmp/baz directory will be shown since we just joined it.

5. If /tmp/baz is a file we can get access to it by using different
commands like DELE (delete) and others.


Same problem affects commands like STOR and RETR, allowing an attacker to
retrieve or upload arbitrary system files.
This would be only limited by rights under which the server is running.
Although different Unix ftp servers permit it, in my opinion we should
forbid access to those parts of filesystem which are outside the user's
home directory since we can't consider the *real* user's permissions.


In order to solve the problem, we should check the type of every filesystem
path issued by client and reject the request in case such path is a
symbolic link.

Original issue: http://code.google.com/p/pyftpdlib/issues/detail?id=55

[giampaolo]

From jlo...@gmail.com on November 26, 2007 13:02:22

I don't like the solution of just rejecting all symbolic links. I think we should
simply check the absolute/real path of any user input paths and if the absolute path
is outside the home directory, then return an error response.

This way users don't have to lose the possible benefits of symbolic links, but
they're kept safe from path traversal vulnerabilities.

[giampaolo]

From billiej...@gmail.com on November 26, 2007 13:15:50

Uhm... I agree with you. 
Let's see what Yan Raber thinks about it since he's the one who solved the old path
traversal problem (see issue #9 ).

[giampaolo]

From billiej...@gmail.com on November 27, 2007 13:29:55

Today I passed a part of the afternoon on working on this issue.
As far as I can say I guess that implementing this kind of stuff will be a real pain
since the "normalize" method logic is currently based on strings and not on a "real"
filesystem reference.

Moreover, I thought about eventual recursion problems we could encounter when
following symlinks.
I was thinking about the following scenario:

linux-box$ ln -s 1 2
linux-box$ ln -s 2 1

...which would cause an infinite recursion loop since the two links points to 
each other.

Another problem is writing a reliable test suite for testing this kind of things. It
would require a lot of work, and I'm not even sure if we could be able to re-create
all the test conditions involved.

[giampaolo]

From billiej...@gmail.com on December 03, 2007 21:30:12

Guys, it seems we made it.
The followings changes have been included in commit #185
( 
http://groups.google.com/group/pyftpdlib-commit/browse_thread/thread/9145777076f0c3e1 ):


- Added the new AbstractedFS.checksymlink() method used for checking symlinks
validity. It is invoked when receiving an argument (the pathname) for those commands
which may be affected by path traversal (APPE, CWD, MDTM, RETR, SIZE, STOR, and XCWD).

- Added a new BadSymlink class inheriting from Exception. It is intended for using by
checksymlink method for providing different error messages when checked link is not
valid.

- Included a solid test suite for testing the cases checked by checksymlinks method:
valid links, broken links, circular links, links which points to a path outside the
user's root.


The test suite has been run successfully on my Linux Ubuntu, the only OS I currently
have which supports symlinks.

[giampaolo]

From billiej...@gmail.com on December 05, 2007 05:42:54

- used os.path.realpath to follow symbolic links.

- removed the check for broken and circular symlinks; if a link is broken or circular
the os.* call itself will later raise an OSError exception.

- removed BadSymlink exception class.

Status: Finished

[giampaolo]

From billiej...@gmail.com on December 23, 2007 20:05:57

Labels: Milestone-0.2.1

[giampaolo]

From billiej...@gmail.com on January 06, 2008 06:37:33

Labels: -Milestone-0.2.1 Milestone-0.3.0

[giampaolo]

From billiej...@gmail.com on January 17, 2008 10:10:01

Fixed in version 0.3.0.

Status: Fixed

[giampaolo]

From billiej...@gmail.com on May 02, 2008 11:26:20

Labels: Version-0.2.0

[giampaolo]

From billiej...@gmail.com on October 13, 2008 12:13:14

Labels: Component-Library