Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Path traversal vulnerability in case of symlinks #55

Closed
giampaolo opened this issue May 28, 2014 · 10 comments
Closed

Path traversal vulnerability in case of symlinks #55

giampaolo opened this issue May 28, 2014 · 10 comments
Labels
bug Component-Library imported imported from old googlecode site and very likely outdated OpSys-UNIX Priority-High Security

Comments

@giampaolo
Copy link
Owner

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
Copy link
Owner Author

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
Copy link
Owner Author

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
Copy link
Owner Author

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
Copy link
Owner Author

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
Copy link
Owner Author

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
Copy link
Owner Author

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

Labels: Milestone-0.2.1

@giampaolo
Copy link
Owner Author

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

Labels: -Milestone-0.2.1 Milestone-0.3.0

@giampaolo
Copy link
Owner Author

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

Fixed in version 0.3.0.

Status: Fixed

@giampaolo
Copy link
Owner Author

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

Labels: Version-0.2.0

@giampaolo
Copy link
Owner Author

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

Labels: Component-Library

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Component-Library imported imported from old googlecode site and very likely outdated OpSys-UNIX Priority-High Security
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@giampaolo