Funkwhale security quick scan report

Hello everyone!

As many of you know, we received some backing from NLNet 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 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, 2 are Resolved. The other 2 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.

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.

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.

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 suggested header ("Strict-Transport-Security: max-age=31536000") is present in our config already. We do not use the includeSubDomains option as we don't think this is a choice we should make for our users. Users can add this to their config if they want a little extra security!

That's it for now! We'll try to be quicker with these summaries in future. Thank you for bearing with us 🙏

By @Sporiff in Announcement Mon 17 January 2022