Skip to content
Snippets Groups Projects
Commit 1fee899b authored by Ciaran Ainsworth's avatar Ciaran Ainsworth
Browse files

Add quick scan report summary

parent 51caeaaa
No related branches found
No related tags found
1 merge request!13Add quick scan report summary
Pipeline #18124 passed
Title: Funkwhale security quick scan report
Date: 2022-01-17
Author: Sporiff
Category: Announcement
Slug: security-quick-scan
Summary: In the name of transparency, let's talk about our security audit!
Hello everyone!
As many of you know, we received some [backing from NLNet]({filename}/blog-post-week-2020-04-we-got-funding.md) nearly two years ago. As part of this, we received a free security audit from the NLNet team to help us button down our hatches and keep everything ship shape 🚢
Due to the [change of maintainers]({filename}/funkwhale-is-looking-for-new-maintainers.md) and the subsequent handover of duties, we've not had time to publish the results of this report and the actions we took to address the issues raised. Since things have calmed down a bit, let's go through these here!
You can read the full report [here]().
## Issues and actions
NLNet's security report involved a quick penetration test performed by an independent security company. Its goal was to point out common exploits present in our app. The report highlighted **4** issues. Of these, **3** are **Resolved**. The last issue we have decided we are **Not doing**. Let's get into the details.
### Input validations are not secure enough
#### The issue
NLNet highlighted that the input validation for some of our endpoints was not secure enough, which might leave them open to server side request forgery. Their suggested action was to introduce stronger validation.
#### Our response
After some investigation we determined that the lack of security in this situation is actually a quirk of federated services. The endpoint that was highlighted by the report is one used to search for remote libraries. This endpoint passes a URL to the Funkwhale API which then sends a request. Since there is no centralized database for us to validate URLs against, there is no way of knowing whether the domain name is a valid Funkwhale URL. The only mitigation would be to implement URL pattern checks, but this would not prevent an attack.
* Decision: **Not doing**.
* Issue link: <https://dev.funkwhale.audio/funkwhale/funkwhale/-/issues/1491>
### Login screen vulnerable to open redirect
#### The issue
NLNet noted that the `/login` screen accepts a `next` parameter. This is used to redirect users to the page they were trying to access after a successful login. However, NLNet noted that this left the user vulnerable to an open redirect, where an attacker could put any URL in this parameter to forward users to a hostile domain.
#### Our response
This was a good catch! We decided to introduce a mechanism that checks the URL in the `next` parameter to ensure it is an internal URL. This way, the user can only be forwarded to screens in the same app they came from. We do this by checking the URL against the list of valid routes in the application, and routing the user to the home page if the URL is not in the list.
* Status: **Resolved**.
* Issue link: <https://dev.funkwhale.audio/funkwhale/funkwhale/-/issues/1492>
### Moderation endpoints do not implement access control
#### The issue
NLNet highlighted that moderation endpoints such as `/manage/moderation` and `/manage/users` were not protected by user access controls. While conditional rendering meant that no content could be accessed by an unauthenticated or unprivileged user, it is best practice to lock the screen itself down.
#### Our response
To make sure users didn't access these screens, we introduced `beforeRouteEnter` guards on all moderation pages. These guards check that the user is authenticated and has the proper rights to access the page, and reroutes them to the home page if they don't.
* Status: **Resolved**.
* Issue link: <https://dev.funkwhale.audio/funkwhale/funkwhale/-/issues/1494>
### Improve security headers in default Nginx config
#### The issue
NLNet suggested that we implement improved security headers in our default Nginx configuration. Better headers improves the security of the web server for all users.
#### Our response
We've implemented several improvements to our Nginx config since this report was done. Notably, the header they suggested we use was already present in our config, so no action was required.
* Status: **Resolved**.
* Issue link: <https://dev.funkwhale.audio/funkwhale/funkwhale/-/issues/1493>
That's it for now! We'll try to be quicker with these summaries in future. Thank you for bearing with us 🙏
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Please register or to comment