Fixed Code Execution bug on fsociety

[d3m0n-r00t]

📊 Metadata *

Remote Code Execution vulnerability

Bounty URL: https://www.huntr.dev/app/bounties/open/1-python-fsociety

⚙️ Description *

Fixed code execution vulnerability by sanitizing the input. Previously the input was raw_input(). Fixed this by splitting the input at spaces (' ').

💻 Technical Description *

Fsociety had many instances where the input was executed as it is by the os.system code. The user input was the target variable which must be an IP address or a domain. But the user was able to execute arbitrary code by adding command line operators such as && or || etc. I splitted this input and made it a way that proper nmap script runs only when an Ip or domain is given as input. It ignores every other inputs.

🐛 Proof of Concept (PoC) *

Vulnerable code:

 def run(self):
        clearScr()
        print(self.nmapLogo)
        target = raw_input(self.targetPrompt)
        self.menu(target)

logPath = "logs/nmap-" + strftime("%Y-%m-%d_%H:%M:%S", gmtime())
        try:
            if response == "1":
                os.system("nmap -sV -oN %s %s" % (logPath, target))

Steps to reproduce the bug:

1. Run python fsociety.py
2. Select any vulnerable part. (For example 1. Information gathering -> 1. Nmap
3. Supply any Ip or domain and with the command line operator supply the payload

127.0.0.1 && echo 'Hacked' > hacked.txt

4. We can see hacked.txt file created in the directory.
   
   

🔥 Proof of Fix (PoF) *

Fix:

target = raw_input(self.targetPrompt).split(' ')[0]

👍 User Acceptance Testing (UAT)

Just splited the input so that only the first parameter of the input is taken. Since IP or domain is considered as a single string it is passed through and the rest is splited out. If any thing other than an Ip or domain is supplied it shows an error showing 'unknown host'. So it doesn't break the code.

[Mik317]

Splitting the target which is a user supplied input doesn't prevent the usage of some commands, which can still be injected and lead to different issues like DOS or unexpected behaviors.
More information have been provided specifically for the lines of code interested and a video had been attached to confirm this (sorry for the low quality of the video ... GIF converter lowers it).

Looking forward for seeing other countermeasures in place to prevent these issue 😄

Regards,
Mik

[Mik317]

Splitting the hostname or the url, which are user-supplied inputs doesn't prevent malicious attackers from injecting successfully other commands like ls or shutdown, which could provoke anyway a DOS issue or unexpected behaviors.

The target should be sanitized properly 😄

[Mik317]

You can confirm what I'm saying just inputting as target the following url: test.cominvalid&&ls , and you'll receive in the final output also the ls command execution, which shows the file in the directory.

Video attached:

[Mik317]

Splitting != sanitizing inputs

[Mik317]

Splitting != sanitizing

[d3m0n-r00t]

Thank you Mik for showing that bypass.
I have added a few code to resolve this issue. Rather than looking for unwanted things in the input I found that checking whether the input is an IP/domain is enough (Since every input variable is an IP/domain). The socket library is imported in the code by default. Socket has a function gethostbyname() which returns IP address if the domain/IP address is valid. Combining with the previous fix now the code takes only the first string and checks whether the string is an IP/domain. If yes then only the code executes.



Please verify the fix.

[d3m0n-r00t]

The fix had a problem with IP subneting. Previous fix had a bypass when an IP range is given
192.168.1.1/30&&ls. Now fixed. By verifying both IP and the subnet range. (Checks whether the range is an integer or not). IP ranges is supplied as an integer (192.168.1.1/24). Thus checks whether the part after '/' is an integer or not thus fixing the bypass.

[Mik317]

Hi @d3m0n-r00t 😄 ,
I reviewed the changes and I think the security issue should be fixed correctly. That said, there are some other occurrences of

os.system('command %s' % (user_supplied_input)))

which should be fixed, like these ones:

• https://github.com/d3m0n-r00t/fsociety/blob/master/fsociety.py#L737
• https://github.com/d3m0n-r00t/fsociety/blob/master/fsociety.py#L1033
• https://github.com/d3m0n-r00t/fsociety/blob/master/fsociety.py#L1058
• https://github.com/d3m0n-r00t/fsociety/blob/master/fsociety.py#L1079
• https://github.com/d3m0n-r00t/fsociety/blob/master/fsociety.py#L1103
• https://github.com/d3m0n-r00t/fsociety/blob/master/fsociety.py#L1140
• https://github.com/d3m0n-r00t/fsociety/blob/master/fsociety.py#L1146-L1147
• https://github.com/d3m0n-r00t/fsociety/blob/master/fsociety.py#L1349
• https://github.com/d3m0n-r00t/fsociety/blob/master/fsociety.py#L1357

I wanted also point out your attention on the fact the fix could break in certain cases the functionality of wpscan and cmsmap, in fact the official documentation of those 2 tools specify the target url/IP can be passed with eventual paths (maybe a WP blog is mounted on https://example.com/blog/, which would lead to a exception since / is splitted for CIRD ranges)

.

That said, I really appreciate your quick fix and the fact you didn't include any new library, which is an important thing in my opinion since doesn't introduce eventual issues/vulnerabilities coming from modules/libraries outside the ones the original author of the code wanted to include in ❤️ .

Regards,
Mik

[d3m0n-r00t]

Hi,
Thanks for the information. I have added a fix to the wpscan and cmsmap functionality issue. As it require a valid url for its working, i have added code to validate the url passed to it. The urllib2 library is imported by default (again), which has a urlopen() function. I have made use of this function to verify the input (after the split), is a valid url or not. I guess now its working fine. Please verify this and inform me if any issue found. And as for the rest of the occurrence, till now I have worked on the basis of the 'Premalink' that was given in huntr (https://www.huntr.dev/app/bounties/open/1-python-fsociety). I will check other occurrences and fix it as soon as possible. In the mean time please verify the current fix and also let me know that the fix is good or not (it's my first).
Thank you

[Mik317]

Thanks again for your help 😄 ,
however I'm still able to bypass the checks in the wpscan/cmsmap functionalities through a special crafted url (more info on the comments).

Regarding the permalinks, I should have linked all of them through the disclosure process as you said, but part of them were still untested by me, so I didn't add them at the begin. That said, the same working solution you'll find for these 3 options will probably fix also the other options as well, so it shouldn't require additional checks or solutions 👍

Regards,
Mik

[Mik317]

Hi 😄,
thanks again for the changes 👍

The characters used inside a valid url are also evaluated as valid bash directives, which leads anyway to arbitrary command execution in minor cases.
The example below is made using the http://evil.com?||nslookup PoC payload, and can bypass the protections applied through urllib2.urlopen().

(please excuse the upper case when typing ... I hadn't clicked the right key)

I understand it's difficult fixing this type of issue, so I think it's ok using also an external library to validate the inputs obtained and avoid arbitrary command injection 👍

Regards,
Mik

[Mik317]

Same above: the urllib2.urlopen checks if the url is valid, but a specially crafted input can still be dangerous and lead to arbitrary command injection.

Mik

[d3m0n-r00t]

Hi,
I have added fixes to all the occurrences to os.system command that executes user inputs. As you told earlier similar fixes is used at verity of places. I will explain each fix method.

For IP/domain

I have made used of the socket.gethostbyname() function as a fix as used for in the nmap part to verify the input is a IP/domain

For URL

Some of the tools require url as an input as their documentation says. Like cmsmap and wpscan. This, even with the previous fix, was vulnerable to specially crafted urls. So as a fix for this, I changed the verification of url using urlparse . I observed that with urlparse the url can be splitted to verious fragments (scheme, netloc, path, query etc), and in the prevous payload the bash command, ie nslookup, comes under the query part. And as the documentation of the tools says the the input url should not include a query. So I splitted the url using urlparse, verified the domain using socket.gethostbyname() and then combined the url thus removing the query part.

test_target = urlparse(target)
socket.gethostbyname(test_target.netloc)
target = test_target.scheme + '://' + test_target.netloc + test_target.path

NB: Fortunately for my luck and since the script is huge urlparse was also imported by default.

For file inputs

Some tools require a file as an input that contains various IPs. So I just added code to check if the file exists or not. The code exists only if the file exists.

os.path.exists(target)

For android hash

I checked the documentation of the tool to crack the pin of android device and its takes a hash and salt as input. But this hashes/salt does not contain any symbol. So added a filter to avoid any symbols.

['!','@','#','$','%','^','&','*','(',')','-','=','+','|','||','&&','/','//','+']

For tool for ftp authentication bypass

For the ftp authentication bypass tool, it takes in 2 inputs with space. First one is the host/IP which is verified by socket.gethostbyname() and the second is the command for the tool. I checked the tool and looked for commands. The tool takes in 3 commands (list, get, and post). So i added a filter for this commands only for the second input.

['get','put','list','GET','PUT','LIST']

I guess that is every occurrences and I guess I have fixed each of them. I have tested the fix and its and the tool did not break till now. Please verify this as it is huge script and I may have missed many. Also verifiy the fix for wpscan and cmsmap.
Thank you

[Mik317]

Hi @d3m0n-r00t 😄
this is awesome!!!!

I found only one thing you should change (the type of URL a tool was set to) and a error in the syntax which prevented from checking (I put a probably fix for that, but lemme know if you have better ideas on that). You'll find more informations in the specific commits.

That said, I think we're close to the final solution, so thanks a lot again ❤️

Regards,
Mik

[Mik317]

Arachni accepts the url like http://example.com as per doc: https://github.com/Arachni/arachni/wiki/Command-line-user-interface#examples.
The socket.gethostbyname() function will raise a exception in case the user provides a url like that.

[Mik317]

The line arises a exception which prevents the user to make working correctly the tool:

. I would suggest to add something like the following line of code:

key = raw_input("Enter the android hash: ")
salt = raw_input("Enter the android salt: ")
symbols = ['!','@','#','$','%','^','&','*','(',')','-','=','+','|','||','&&','/','//','+', ' '] #Added ' ' to avoid split(' ') which turns `key` and `salt` in a `list`

[symbol for symbol in symbols if symbol not in key and symbol not in salt] == symbols # if True --> the `key` and `salt` strings are correct (don't match any of the `blacklisted` chars)

This makes a list comprehension which therefore checks if the string contains one of the characters blacklisted returning the list resulting and then checks if it's equal to the one predefined before ( symbols ).

[d3m0n-r00t]

Hi,
Thanks a lot @Mik317 for informing me about that url parse error. I have added the fix for url for it. I hope it is correct. And about the android hash it was a stupid logical error. Sorry for the mistake and thank you for the fix too. I found your fix quite unusual but effective thanks. Have added it.
I also found another occurrence of this type of bug. There was an inurl function with similar problem. So I went and looked the code of the tool and found an array of all the dorks that can be possibly used for the tool. I have added code to check whether the dork is in the array or not.

all_dorks = ['dork:', 'dork-file:', 'exploit-cad:', 'range:', 'range-rand:', 'irc:', 'exploit-all-id:', 'exploit-vul-id:', 'exploit-get:', 'exploit-post:', 'regexp-filter:', 'exploit-command:', 'command-all:', 'command-vul:', 'replace:', 'remove:', 'regexp:', 'sall:', 'sub-file:', 'sub-get::', 'sub-concat:', 'user-agent:', 'url-reference:', 'delay:', 'sendmail:', 'time-out:', 'http-header:', 'ifcode:', 'ifurl:', 'ifemail:', 'mp:', 'target:', 'no-banner::', 'gc::', 'proxy:', 'proxy-file:', 'time-proxy:', 'pr::', 'proxy-http-file:', 'update::', 'info::', 'help::', 'unique::', 'popup::', 'ajuda::', 'install-dependence::', 'cms-check::', 'sub-post::', 'robots::', 'alexa-rank::', 'beep::', 'exploit-list::', 'tor-random::', 'shellshock::', 'dork-rand:', 'sub-cmd-all:', 'sub-cmd-vul:', 'port-cmd:', 'port-scan:', 'port-write:', 'ifredirect:', 'persist:', 'file-cookie:', 'save-as:']

And I guess that is all. Thanks again.

[Mik317]

Hi @d3m0n-r00t 😄
I think we fixed massively the repo and I can confirm the tool seems work correctly and all the instances of os.system() are taking properly sanitized inputs.

Regarding the solution of the symbols list, I know it's not really good, but it's the only way I found which simply resolves the issue (and I ❤️ list comprehension).

So, I'm approving this fix and in the meanwhile I ask @mufeedvh or @toufik-airane to check if the code seems fixed for you also 👍 .

Thanks again for your help and the amazing collaboration, we ❤️ this!!!
Looking forward to see other fixes/disclosure by you on the https://www.huntr.dev website 😄 🎉

Regards,
Mik

[d3m0n-r00t]

Thank you @Mik317, I too enjoyed this collaborative fix with you. Thanks. I liked your approach on the symbols list, it is a wonderful way to check for blacklists. (Good to learn new things).
Thanks.

[Mik317]

My pleasure @d3m0n-r00t 😄
I had a hard time trying to bypass the excellent fixes you made, so I enjoyed it a lot 😂 too.

Cheers,
Mik

[mufeedvh]

Woah, Awesome fix! 👏🎉

No new dependencies, just pure code! 💯

Loved the back and forth collaboration between you and @Mik317 ❤️

Keep your fixes coming! 🔥

[JamieSlome]

Great work all!

We will be featuring this PR on our Twitter today as a great example of security in the open! 🍰

[huntr-helper]

Congratulations d3m0n-r00t - your fix has been selected! 🎉

Thanks for being part of the community & helping secure the world's open source code.
If you have any questions, please respond in the comments section. Your bounty is on its way - keep hunting!