dont expose the stacktrace

This commit is contained in:
Wouter Groeneveld 2022-05-29 11:44:31 +02:00
parent d91e1b7563
commit 1eb04ea89d
2 changed files with 47 additions and 1 deletions

View File

@ -0,0 +1,46 @@
---
title: "Don't Expose The Stacktrace Please!"
date: 2022-05-29T11:14:00+02:00
categories:
- programming
---
The other day, we were looking for nearby flea markets to attend---it's starting to [become flea market season](/post/2021/06/flea-market-season), rejoice! I remembered an ad when biking around our neighborhood but couldn't remember the exact location (or the location of the ad itself). The internet to the rescue. Search engines suggest barely working websites without HTTPS that list markets from 2019---next to the obligatory Facebook group, which is a sure way to exclude people without Facebook accounts from attending your thing. I really hate that trend.
One of those sites exploded. _BOOM_. It spat out... an exception?! I'm always intrigued when encountering errors on the web besides the dry "404" messages, so I took a better look.
`Call to undefined method Illuminate\Support\Facades\Request::session()`
Okay.
At `app/Http/Controllers/HomeController.php:159`:
```php
// ... more PHP code
$suggestions = DB::select("SELECT events.id, categories.nl_category, eventname, DATE_FORMAT(dates.startDate,'%d/%m') AS startDateEU, city,provinces.province, ABS( LEFT(zip,4) - $eventZipClean) AS closest
FROM events
JOIN addresses on events.id = addresses.event_id
JOIN dates on dates.event_id = events.id
JOIN details on details.event_id = events.id
JOIN provinces on addresses.province = provinces.id
JOIN categories on categories.id = details.category
WHERE dates.startDate >= curdate() and dates.startDate <= ( curdate() + INTERVAL 1 MONTH) and events.approved = 1 and events.id != $event->eventId
ORDER BY closest
LIMIT 4");
if(Carbon::createFromFormat('d/m/Y', ...
$req->session()->flash(...
```
Wait, what? After the initial shock, I was even presented with `TODO` comments in the PHP code a few lines below the query. The zip code system apparently still has to be implemented.
A tab "Context" revealed Laravel version 8.28.1 and PHP version 8.0.19. There's even a "Debug" tab that neatly summarizes all sql queries together with the connection name and how long it took to execute them. _There's even a "Share" button_. A share button! Perhaps they should also add the whole slew of social media icons to share it to.
This stacktrace is so wrong on so many levels. Why am I, as a visitor of your site, when something goes wrong, presented with technical data---that completely exposes the way your backend works, your database is built, and your code is structured? As a side note, dear developer, did you know that stuffing queries in the controller isn't the way we do things in 2022? That `TODO` item was just too hilarious.
A stacktrace is not a vulnerability in itself. But as you can see here, it clearly leaks a lot of information---_internal_ information that can be easily abused by an attacker. Exposed queries are an open invitation to try out a few SQL injections, and even tough the `$eventZipClean` variable is called "clean", it could very well not be.
When it comes to security issues, PHP has the reputation of being as holey as a Swiss cheese wheel. You can clearly see why. This isn't entirely PHP's fault though, and neither Laravel's: [according to the Laravel 8.x docs](https://laravel.com/docs/8.x/errors#configuration), it has an `APP_DEBUG` environment variable that should always be set to `false` in production---emphasis is added in the docs. Clearly, the developers didn't read this. The fact that PHP is an interpreted scripting language that was heavily used in the nineties to hack together a few silly things for your website doesn't exactly help, and I'm using the verb "hacking" in a negative context here.
Please, _please_ be careful with debug configuration and information in a production environment. If you still want that info, log it somewhere else, present the user with a unique ID and ask them to contact support, gracefully degrade, show a generic message, redirect, whatever, but don't expose the stacktrace. Please.

File diff suppressed because one or more lines are too long