avkpol / pulsarvoip

0 stars 2 forks source link

Shell injection vulnerability #1

Open progval opened 8 years ago

progval commented 8 years ago

I think there is a shell injection vulnerability here: https://github.com/avkpol/pulsarvoip/blob/bd892833cbcb7164093c2a15e6f5382ca7edef3e/src/pulsarvpn/views/button_handler.py#L23-L27

The user name is substituted in the command without any sanitization. A possible fix is to rewrite the code this way:

    openKeyGen = ['openvpn', '--genkey', '--secret', 'key_' + req_user]
    openKeyTest = ['openvpn', '--test-crypto', '--secret', 'key_' +req_user]
    if request.method == 'POST':
         previousDir = os.getcwd()
        os.chdir('/etc/openvpn')
        try:
            subprocess.Popen(openKeyGen)
            subprocess.Popen(openKeyTest)
        finally:
            os.chdir(previousDir)

And similarily for the rest of the code (which is currently commented)

avkpol commented 8 years ago

Happy New Year!! Thanks, Valentin, for comment and stuff to learn. This way code looks much more “pythonic” Andriy

On Jan 1, 2016, at 2:39 PM, Valentin Lorentz notifications@github.com wrote:

I think there is a shell injection vulnerability here: https://github.com/avkpol/pulsarvoip/blob/bd892833cbcb7164093c2a15e6f5382ca7edef3e/src/pulsarvpn/views/button_handler.py#L23-L27

The user name is substituted in the command without any sanitization. A possible fix is to rewrite the code this way:

openKeyGen = ['openvpn', '--genkey', '--secret', 'key_' + req_user]
openKeyTest = ['openvpn', '--test-crypto', '--secret', 'key_' +req_user]
if request.method == 'POST':
            previousDir = os.getcwd()
            os.chdir('/etc/openvpn')
            try:
        subprocess.Popen(openKeyGen)
        subprocess.Popen(openKeyTest)
    finally:
                os.chdir(previousDir)

And similarily for the rest of the code (which is currently commented)

— Reply to this email directly or view it on GitHub.