DEV Community

Richard Klose
Richard Klose

Posted on

Learn from others mistakes: How not to write a PHP install script

#php #vulnerabilities #security #https

I was playing around with different photo library software latetly, mainly because I was looking for something I could selfhost in my home network. I was using Apple Photos and Lightroom before, but this always became a difficult part as soon as I wanted to share my photo library with my wife, including all tags, annotation and metadata.
I stumbled over koken and wanted to give it a try.

Koken is not open source but written in PHP, so it is more or less easy to investigate the code and have a look under the hood. Its so called "Installer" is just a PHP script with some HTML and JavaScript, so I started there.

The last modified date of the "Installer" file is 2017-02-24, which already made me wonder if it's up to date according to security. Having a look inside I discovered it's definitly not.

Outdated jQuery (1.8.0)

The first step of the installation process is a server compatibility check. This is triggered by a HTML button click, that executes a JavaScript function called test(). This function uses jQuerys $.post() function to trigger the PHP part of the server compatibility check.

...
<script src="//ajax.googleapis.com/ajax/libs/jquery/1.8.0/jquery.min.js"></script>
...
Enter fullscreen mode Exit fullscreen mode 
function test() {
  ...
  $.post('index.php', {
    server_test: true,
    custom_magick_path: magick_path
    }, function(data) {
      ...
    }
  }
  ...
}
Enter fullscreen mode Exit fullscreen mode

The jQuery version that is used here is 1.8.0 which is not only 6 years old by now, but also has security vulnerabilities known since January 2018. The latest 1.x version of jQuery at 2017-02-24 (the last modified date of the script) was 1.12.4 which at this point already was half a year old. I would expect at least a non-outdated version like 1.12.4, but as this is also known to have security vulnerabilities, better jQuery > 3.0.0 should be used.

Script itself must be writeable

On the server side, the server_test variable from the client is evaluated by PHP:

if ($_POST)
{
  if (isset($_POST['database_check']))
  { ... }
  else if ($_POST['server_test'])
  {
    $current_dir = dirname(__FILE__);
    $writable = is_writable($current_dir) && is_writable(__FILE__);
    ...
    if (!$writable || (...))
    {
    $permissions['fail'][] = 'This directory does not have the necessary permissions to perform the install. Set the permissions on the koken folder and the koken/index.php file to 777.';
    }
    ...
    }
    ...
}
Enter fullscreen mode Exit fullscreen mode

So the first thing that this script checks is whether the current directory and the script file itself is writable by the user that runs the webserver.
Although it totally makes sense that the folder is writable, as koken will be installed here, the script itself should definitly not be writable. Enforcing this means any process that runs with the same user can modify the script at any time. We will see later, that all the download URLs are hardcoded in the script, so they now can easily be modified and pointed to some malicous URL. Of course the script can also be modified to do whatever is possible with PHP.
I assume making the script writable is necessary as it is called index.php and may later be replaced with koken's real index.php. A better solution here would be to give it another maybe more descriptive name like install.php and force the user to delete it manually after the installation. This would not only ensure that the install script can be non-writable but also would help the user to understand a little bit better what's going on here.

Weird download

The very next step during the server_test is a download of PclZip (A PHP library for handling zip files).

list($download, $download_error) = download('https://s3.amazonaws.com/koken-installer/releases/pclzip.lib.txt', 'pclzip.lib.php');
Enter fullscreen mode Exit fullscreen mode

That's really strange, because the purpose of the server_test code should be, to test if the server fulfills the requirements. But why is this downloading stuff, before anything truly relevant (e.g. the PHP version) has been tested?
It's also strange, that PclZip is not downloaded from the official release downloads but from koken's aws. Comparing koken's version and the official one reveals that it is modified, but the only change is the removal of empty lines and the closing PHP tag (?>) at the end of the file.
Side note: The version of PclZip they use here (2.8) is also outdated. Thats noteworthy because the following version (2.8.1) includes a fix for PHP 5.3 which is koken's minimum required PHP version, so it totally makes sense to upgrade here.

Weird download enables MITM attacks

By looking deeper into the previous seen download function, I found the following lines:

curl_setopt($cp, CURLOPT_FOLLOWLOCATION, true);
curl_setopt($cp, CURLOPT_SSL_VERIFYPEER, false);
Enter fullscreen mode Exit fullscreen mode

This means the HTTP Request for the download of PclZip (and probably every other download that will be done later during the installation) follows every HTTP 3xx redirect while not verifying the authenticity of the server that responds. Or in other words: "I'll download whatever I find behind this URL, I don't care who you are, I'll take the file anyway, even if you serve me some malicous stuff". This opens the door for man-in-the-middle-attacks.
Especially nowadays, where you can get SSL certificates for free, there is no need to disable this verification. Just don't do this.

Checking browser far too late

This is not a security isssue but more a usability problem.

... // the previous mentionend download of PclZip
$ua = strtolower($_SERVER['HTTP_USER_AGENT']);
... // several testing stuff here
if ( strpos($ua, 'msie') !== false || strpos($ua, 'internet explorer') !== false)
{
  $browser['fail'][] = 'The Koken beta does not currently support Internet Explorer. Please use Chrome, Safari or Firefox.';
}
Enter fullscreen mode Exit fullscreen mode

It's okay to not support Internet Explorer, but this test can already done by a piece of JavaScript in the browser itself before any communication between the client and the server took place. This way, the script has already downloaded stuff onto your server and then enforces you to restart the process, just because you used the wrong browser.

More insecure HTTP(S)

Later during the server_test:

$loopback_test = koken_http_get('/json');
Enter fullscreen mode Exit fullscreen mode 
function koken_http_get($url, $host_header = false)
{
  ...
  if ($protocol === 'https')
  {
    curl_setopt($curl, CURLOPT_SSL_VERIFYHOST, 0);
    curl_setopt($curl, CURLOPT_SSL_VERIFYPEER, false);
  }
  ...
}
Enter fullscreen mode Exit fullscreen mode

Seriously, people at koken: Please do not disable SSL verifications. This enables man-in-the-middle attacks. Get a free SSL certificate instead and prevent your users from being hacked.

Conclusion

Just by looking into the install script of koken, I got a sneak peak of how the people at koken deal with security. They seem to not really care about it. I found out:

  • They use outdated dependencies with known bugs and security vulnerabilites.
  • They force the user to have a PHP script on their server writable by the user that runs PHP, so another PHP script could modify it.
  • They already download stuff before they know if koken will run on the server at all.
  • They disable the verification of SSL certificates for their downloads during installation.

There is much more going on in this script, but I'll stop here as these are the things that made me doubt most.

If this is the way people at koken write software I really cannot recommend using koken. Even if it might be a nice software for photographers, this is such a security flaw for your server.

I asked them on twitter about this a few days ago and got no response.



In combination with a last modified date from more than a year ago and also no activity in their blog since a year I have truly no hopes that this situation will change.

My plea to all of you developers: Please care more about security and don't make your (probably good) software fail by doing without.

Top comments (2)

Collapse
 
antogarand profile image
Antony Garand •

Did you send them an email regarding those first?

Although there are no "real" vulnerabilities disclosed, it is a bad practice to write about this without letting the author the change to fix the issues.

Collapse
 
richard_klose profile image
Richard Klose •

I tried to contact them on Twitter but they just ignored me. Thought they might not have seen it but they are responding to other people‘s tweets so I think they just do not want to hear this.

I am not the only one who found issues, if you search around in photography forums you‘ll find several other things people are unhappy with about koken. The result is nearly every time the same: People that report issues to koken are often ignored.

I thought about sending them emails and even about calling them directly, but when I read those reports from other people that tried to report issues, I decided to do not and publish this because I do not believe this would change anything in this situation.

Read next

unicodeveloper profile image

The Ultimate Guide to Laravel Reverb

Prosper Otemuyiwa -

leandro_nnz profile image

Bringing Security to the Forefront of DevOps: The Power of Threat Intelligence Integration

Leandro Nuñez -

richardshaju profile image

Why is PHP still relevant in 2024?

Richard Shaju -

mochafreddo profile image

Understanding Cross-Site Scripting (XSS)

Geoffrey Kim -