Securing a Bad Blogging Platform Dec. 1, 2016 in Cybersecurity, Php, Text, University, Web
I was tasked with securing $BloggingPlatform. Here are my findings.
SQL Injection
What vulnerability did you find and where was it present?
There were many different SQL Injections located all over the site, caused by blog.php, authhelper.php, and genericmodel.php. In blog.php and authhelper.php you found custom queries, none of which do anything to prevent SQL injections. In genericmodel.php the functions expect arrays in their parameters, but did little to check. The fetch method did check for the input being an integer but this wasn’t enough.
How did you resolve this vulnerability?
I fixed blog.php and authhelper.php by rewriting the queries as prepared statements, now the query and the data are stored and processed separately, preventing such attacks. I fixed genericmodel.php by adding what are essentially guards, which either wrap the input or throw an exception depending on what goes wrong.
This essentially prevents all SQL injections.
Information Exposure
What vulnerability did you find and where was it present?
$BloggingPlatform had many temporary, unused files that can contain sensitive information. Here’s the full list:
.bash_logout.bashrc.index.php.swo.profile.tarignoreCHANGELOGinstall.sqlsetup.phpVERSIONcontrollers/contact.php.bakcontrollers/controller.phpsconfig/db.cfg.bakmodels/.PagesModel.php.swputility/Debug.php
These files simply shouldn’t have been there, accessible to the world-wide web. It’s kind of remarkable how many different types of file there were. Bash profile files, change-logs, installation scripts, vim backup files, manually created backup files, multiple versions of the same PHP script.
By default, a lot of files are accessible that shouldn’t be, this includes db.cfg which contains the database passwords. If the server wasn’t hosting PHPMyAdmin, the application wasn’t vulnerable and a firewall was in place to block external access to the database this wouldn’t be the end of the world, but in $BloggingPlatform’s initial state, this gave any attacker complete control over your database and thus your site.
How did you resolve this vulnerability?
I fixed the first two issues by firstly deleting those unwanted, mostly temporary files and adjusting the .htaccess files. By default, they restricted access to *.ini files and redirected most traffic to index.php. This wasn’t good enough for me, I wanted to redirect all traffic to index.php regardless of errors or if any files exist (Except for static and uploads, where I put RewriteEngine Off to disable this traffic redirection). This is achieved with RewriteRule ^.*$ index.php [L,QSA], and ErrorDocument’s for each error.
Cross Site Scripting
What vulnerability did you find and where was it present?
I found that virtually every single time the user can input data, there existed an XSS vulnerability. There was absolutely no care put into handling user output, anyone who bothers to register an account could easily flood the site with XSS exploits. I accidentally did using BURP!
There was also a comments box, and the posts/pages editor which sent raw HTML to the server, fixing this was more difficult because you can’t just run it through h(), that will destroy the markup.
How did you resolve this vulnerability?
I noticed that inside functions.php was the function h() which is a shorthand for PHP’s htmlspecialchars(), a useful function that converts potentially harmful characters such as < into harmless escape sequences such as \< these can be safely echoed onto the page.
To fix the comment box I used http://htmlpurifier.org, a “standards-compliant HTML filter library written in PHP”, by default it filters raw HTML through an audited white-list to protect against XSS attacks. This library works great, all I had to do was copy it into my utility folder, add an import in bootstrap.php, and now whenever I need to filter HTML for XSS, it’s one line of code.
Insecure Upload
What vulnerability did you find and where was it present?
I found an upload form in the comments/posts/pages WYSIWYG editor, and one in the profile editor where you can change your avatar. They allowed you to upload any file, with any mime-type and any extension to the server, where it is placed inside the uploads folder. If you name it an existing file name then it overwrites it, so you could replace pictures in posts, comments, avatars, or comments.
But even worse, you could upload a PHP script and the server would happily execute it, no questions asked. With this you could easily run whatever you want, install a reverse shell, clone the site, access the database, etc. Without any validation, this had the capability to break the entire site.
I also found that the uploader didn’t care if you are logged in or not, I changed that.
How did you resolve this vulnerability?
I first check if the user is logged in. Then inside file.php, I limit the size of image uploads to 2MB, and check that the image is either a *.bmp, *.gif, *.jpeg, or *.png. Using PHP GD I now strip the metadata from them to protect users who accidentally upload sensitive information, including GPS coordinates in the metadata. I don’t remove metadata from GIFs as it would break the animation. Lastly I SHA256 hash the file, and ensure the name is unique with an offset. Users can’t overwrite files, even if a SHA256 collision is found.
The second approach I took was by editing the .htaccess files. I had already redirected virtually everything to index.php making the uploads and static folders inaccessible, however you need to be able to view the content such as style sheet, javascript, and the uploads. To fix this in a new .htaccess I put the following:
RewriteEngine Off
<Files *>
SetHandler default-handler
</Files>
The first statement disables the RewriteEngine, so the directory is visible to the outside world. The second sets the file handler of every file in this directory and any child directories to the default; this has the effect of disabling PHP, or any other scripting language the server is running. This defense in depth means that even if my uploading script fails, uploads will not be executed.
Cross-Site Site Request Forgery
What vulnerability did you find and where was it present?
Every single user input in $BloggingPlatform was vulnerable to CSRF because $BloggingPlatform didn’t provide any CSRF protection at all. There were easy to exploit GET requests that change state such as /blog/moderate/[0\|1], /admin/blog/delete/[id], /admin/category/delete/[id], /admin/page/delete[id], and /admin/user/delete/[id]; and slighter harder but still exploitable POST requests all over the site.
Ideally you never want to change state on a GET request, otherwise you are breaking the semantics of HTTP. If $BloggingPlatform had kept all state changes to POST requests, fixing CSRF would have been easier.
How did you resolve this vulnerability?
Securing POST requests was easy. In authhelper.php->setupSession() I assigned each user a unique 32-byte CSRF token when they logged on. In form.php->start() I added a hidden CSRF field to every automatically generated POST form. Finally, in controller.php->beforeRoute() I check if debug mode is disabled, and the request is a POST request, and the supplied CSRF token is invalid, finally adding a warning and redirect to site root. This kills all POST CSRF attacks.
Secondly, I needed to fix the GET requests. I figured that the easiest and most secure way to fix this would be converting all GET requests that change state into POST requests. I achieved this by individually wrapping each link in an automatically generated form, and adding a check to ensure it is a POST request and only a POST request. It’s pointless to convert all POST requests if you still accept GET requests.
Authorization Bypass
What vulnerability did you find and where was it present?
There were too many places in $BloggingPlatform without authentication to mention. admincontroller.php only checked if the user was logged in, not if they were an admin, blog.php->comment() didn’t check before deleting or changing the visibility, upload.php didn’t check if the user was logged in, etc.
How did you resolve this vulnerability?
In admincontroller.php, I changed the code to check the users level was sufficient before routing:
if($this->Auth->user('level') < $this->level) {return $f3->reroute('/');}
And everywhere else, I added essentially the same code to the top of the relevant functions. This was relatively easy to fix, the difficulty was finding everywhere that needed it.
Internal Information
What vulnerability did you find and where was it present?
Whenever $BloggingPlatform encountered an error (often), it would display a stack trace to whomever triggered it. This vulnerability allowed an attacker to gain information about how $BloggingPlatform works, what classes and functions there are, all useful information when attacking.
How did you resolve this vulnerability?
I fixed this by wrapping the stack trace in a debug test. I considered showing it for only admins but decided against it - no sane admin would turn on debug in production, they have error logs for that. I remembered I am securing horrible PHP code, and felt bad for overestimating PHP ‘admins’.
Parameter Manipulation
What vulnerability did you find and where was it present?
The contact form let you enter an address to send from, this is silly. User’s already have a registered email address that we should use instead. In addition, the comment form had a hidden field, user_id, that contained the user ID, however as it’s user input, it could be changed to anything, to comment as anyone.
$BloggingPlatform contained a lot of calls to extract() and copyFrom(), which are as bad as eval() if you care about security. Either combined with a way for the user to provide input allowed them to overwrite any variable or user attribute at will. This is because they blindly copy an array, overwriting what’s there.
How did you resolve this vulnerability?
I removed both the form fields, and changed the code to use the user’s information.
Using grep, I scanned through the entire code base and manually removed every single extract() and copyFrom() call, excluding third party libraries. Anything less is a security hole. This took time, but it removes a huge number of potential exploits where the user can either mess with execution, or perform a privilege escalation attack. I also removed two horrible pieces of code in controller.php where an extract() was used just before an eval().
Application Logic
What vulnerability did you find and where was it present?
Draft posts were handled poorly in $BloggingPlatform, while not visible on the front page, everywhere else doesn’t care if they have been published or not. They showed up in the search box, user pages, you could even view them directly! Similarly, unmoderated comments have a habit of showing up.
The API was full of such peculiarities, you could get a full copy of the database, tokens were improperly checked, the use of extract(), the list of problems just went on and on.
How did you resolve this vulnerability?
I added the check published IS NOT NULL so it only shows published posts. I noticed a similar problem with unmoderated comments, and added a check for moderated = 1.
I had to rewrite the API from the ground up. I changed the token authentication to work correctly, and then enabled it in debug mode or when you’re an admin as well. Next I added a white-list of valid calls with checks to only show sensitive information to the user if they are authenticated. I decided to present a sanitized version of the API to unauthenticated users, however an admin could easily change this behaviour.
Out of Date Software
What vulnerability did you find and where was it present?
One of the very first steps I made to secure $BloggingPlatform was to update the core of $BloggingPlatform itself. Looking at the HTTP request in Chrome dev tools yielded no new information, so I checked the source code of any generated page, where I noticed that it included the line:
<meta name="generator" content="\$BloggingPlatform 0.6c - http://BloggingPlatform.clicked.cc"\>
This lead me to suspect that just like RobTheBank, I would find useful information by visiting the link.
How did you resolve this vulnerability?
Once on the page I found 06d.patch, which I downloaded to the correct directory, then quickly applied using: patch --strip 1 < 06d.patch.
While $BloggingPlatform was now up to date, Fat Free was running version 3.5.1, released nearly a year ago, whereas the latest version, 3.6.0 was released this month with an extensive change-log. I updated Fat Free, keeping the original index.php because it hadn’t significantly changed and contained the all-important routes that $BloggingPlatform needs to operate.
Updating $BloggingPlatform to 0.6d wasn’t without problems though, it seems as if the 0.6d patch supplied upgraded the Fat Free Framework from 0.5.0 to 0.5.1 without testing the rest of the application as the stack traces on the error page broke. Fortunately this required a simple fix, replacing the complex PHP array parsing code with: <pre class="well"><?=$ERROR['trace']?></pre>
File Inclusion
What vulnerability did you find and where was it present?
In page.php, a user can include the contents of arbitrary files regardless of extension, the contents of which are passed through eval(). For example, this means that metadata in an image upload can be executed as PHP code which is a horrible, horrible idea.
How did you resolve this vulnerability?
I fixed this code in multiple ways. I firstly removed the eval(), as there is virtually no good reason to include it in a modern PHP code base, especially when the input relies on user input! Next I replaced the code which first checks if the file doesn’t exist and appends “.html” with code that always appends “.html”; this ensures that the file extension is always “.html”. Lastly, I filtered the valid page names using the following regex: /[^\w\-]+/. It’s rather restrictive, but should help protect against malicious input.
Insecure Cookies and Sessions
What vulnerability did you find and where was it present?
$BloggingPlatform ‘helpfully’ set a $BloggingPlatform_User cookie which contained the entire user’s session, serialized and converted to base64. If an attacker decided to decode this string, they could edit it too whatever they want, giving themselves admin powers, and possibly breaking the site. The cookie was horrid!
As it’s only created when the user logs in, while they remained logged in any account changes require the cookie to be deleted (i.e. they manually log off). If you want to change their username, their permission level, or even delete their account you can’t!
Lastly, as the session_id was set to md5(user_id), it meant there are not many unique session_ids, and they were easily iterable. An attacker could simply find a user_id somewhere on the site, or loop until they found one. They then could potentially got an admins session.
How did you resolve this vulnerability?
I started by adding a sessions table to the database, with the columns: id, token, user_id, and created. Next in authhelper.php I added code to create a random token, store it in the database, and create the newly renamed BlogPlatformUser cookie. Lastly, I added code to verify the cookie by retrieving the token, connecting the database to retrieve the session, and finally logging in as the user, if the token is valid. It works well.
I changed the session back to PHPs random default.
Open Redirects
What vulnerability did you find and where was it present?
I found the open redirect using Burp Suite. Leaving it running while I was navigating the site for the first time created what is essentially a site map which was incredibly useful for finding this vulnerability, and confirming it was the only one. The open redirect could be found at /user/login?from=/, where the from parameter is a URL encoded address, which $BloggingPlatform blindly redirects.
How did you resolve this vulnerability?
The layout of $BloggingPlatform meant I knew exactly where to look, /controllers/user->login. Looking at the function I knew not to change it, and saw that the actual redirect code was located in the afterLogin function where it simply calls: \$f3->reroute($_GET['from']);.
I decided that I only wanted to redirect to local links. Rather than writing a flakey regex myself, I searched and quickly found the PHP function parse_url($url) which takes a URL, splitting it into its constituent parts, returning them as an array.
The fix was simple, check if the host key is not set, then redirect to the supplied local URL, otherwise redirect to site root.
Brute Force
What vulnerability did you find and where was it present?
A secure site without a form of brute force protection is not a secure site. Looking at $BloggingPlatform I decided that the login, and register pages needed brute force protection to prevent an attacker simply enumerating passwords until they guess correctly. In addition, I felt that the contact page, comments box and image uploader also needed it, to prevent spam.
How did you resolve this vulnerability?
I decided to use Google’s reCaptcha too not inconvenience the user. reCaptcha is disabled for admins.
Implementing was easy, simply upload recaptchalib.php to the utility directory, define the sites public and private key in config.cfg, and add a couple of lines of code where necessary. Overall, I’m glad I used reCaptcha, even when it asks you to recognize street signs, shop fronts, or rivers.
It’s important to note that adding reCaptcha is only one part of protecting your site from malicious traffic, ideally you want firewall rules and to use tools such as Cloudflare to stop persistent offenders.
Anything Else
What vulnerability did you find and where was it present?
$BloggingPlatform didn’t hash passwords, this was horrible because if your site is compromised an attacker could not only get a complete database, but get a list of raw passwords. People often reuse passwords so a list of passwords is a list of logins to many other sites. It’s only responsible to hash and salt passwords.
How did you resolve this vulnerability?
I added password hashing by editing authhelper.php and usersmodel.php. As of PHP 5.5, PHP now provides a nice, easy to use password hashing and rehashing series of functions that take care of everything. When a user attempts to log in I retrieve their hashed password and use password_verify() to compare, if the user doesn’t exist I compare it against the word “password” hashed to ensure a roughly constant time, regardless of if a user exists.