
Finding an arbitrary file upload vulnerability in a filesharing script - internetwache
https://0day.work/finding-an-arbitrary-file-upload-vulnerability-in-a-filesharing-script/
======
fake-name
> I thought about a possible fix for a while, but in the end decided that the
> quickest and easiest fix would be to adjust the regular expressio

..... WHAT?

Python has a suite of facilities _exactly_ for this very kind of problem.

Literally, the solution is
"os.path.abspath(filename).startswith(os.path.abspath(dlfolder))"

This should, in all cases, return true if the filename is within the download
folder directory, and false for any other case.

~~~
jwilk
It doesn't do the right thing if dlfolder doesn't end with a slash.

------
jwilk
So I was curious what translate_path does:

    
    
        def translate_path(self, path):
            """Translate a /-separated PATH to the local filename syntax.
    
            Components that mean special things to the local file system
            (e.g. drive or directory names) are ignored.  (XXX They should
            probably be diagnosed.)
    
            """
            # abandon query parameters
            path = path.split('?',1)[0]
            path = path.split('#',1)[0]
            path = posixpath.normpath(urllib.unquote(path))
            words = path.split('/')
            words = filter(None, words)
            path = os.getcwd()
            for word in words:
                drive, word = os.path.splitdrive(word)
                head, word = os.path.split(word)
                if word in (os.curdir, os.pardir): continue
                path = os.path.join(path, word)
            return path
    

That... that doesn't make any sense.

~~~
jwilk
Oh, and thanks to urllib.unquote, I can disguise slashes as "%2f", so the
tightened regexp doesn't help.

~~~
jwilk
Never mind me, I misread how this function is used.

Apparently this code was copied from stdlib, including a different directory
traversal bug on Windows:

[https://bugs.python.org/issue26657](https://bugs.python.org/issue26657)

------
jwilk
Apparently the upload directory is the same as the script directory, which is
not good. If you upload a file named "cgi.py", it will be imported next time
someone runs the script. No directory traversal needed to exploit this.

~~~
gehaxelt
Hi,

I asssumed that the script would not be restarted, but you're totally right.

Thanks for the comment :)

------
based2
[https://www.owasp.org/index.php/Unrestricted_File_Upload](https://www.owasp.org/index.php/Unrestricted_File_Upload)

------
netsec_burn
Nice find, I commented on your blog a method you can use to extend this
vulnerability into RCE without admin interactivity.

~~~
gruez
>Upload a .py file to the untrusted directory that has the same name of one of
the imports used in the Python script. It will be executed the next time
Python is called by the script, so you don't need to wait for interactivity on
the admins part

I have a feeling that only works on python2 because of absolute imports on
python3.

~~~
gcr
Upload a script that replaces the old file source and wait for gunicorn or
whatever to reload the app server.

~~~
gehaxelt
The script seems to ship with a rundimentary webserver by default, so I don't
see a reason to wrap gunicorn et al. around it?

However, you're right that overwriting the source is a possible attack vector.

------
ecdh
This is hilarious -- I found this exact script and noted the exact same
vulnerability last year. Good on you for posting about it.

~~~
gehaxelt
Nice catch :)

I thought the the amount of forks and stars warrants a short writeup.

