Security: Raw exception messages are returned to clients in throttling middleware

[tomaioo]

Summary

Security: Raw exception messages are returned to clients in throttling middleware

Problem

Severity: Medium | File: lib/Middleware/ThrottleFormAccessMiddleware.php:L21

In afterException, the middleware returns $exception->getMessage() to the client for NoSuchFormException. If exception messages include internal identifiers, query context, or untrusted input, this can lead to information disclosure and potentially reflected content issues in consuming clients.

Solution

Return a fixed, generic error message (e.g., 'Form not found') and log detailed exception context server-side only.

Changes

• lib/Middleware/ThrottleFormAccessMiddleware.php (modified)

[codecov]

Codecov Report

❌ Patch coverage is 0% with 1 line in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
lib/Middleware/ThrottleFormAccessMiddleware.php	0.00%	1 Missing ⚠️

📢 Thoughts on this report? Let us know!

[pringelmann]

Hi @tomaioo, thanks for the report, but I don't think this one holds up.

I checked every throw site of NoSuchFormException (lib/Service/FormsService.php, lib/Controller/ApiController.php) and all the messages are static hardcoded strings. Nothing gets interpolated from form IDs, user input, or query context, so the "reflected untrusted input" risk in the description doesn't actually exist in the code today.

There is a smaller, related concern: the different messages do give away whether a form is missing vs. forbidden, which chips away at what the throttle is trying to prevent. But the PR only rewrites the body and still forwards $exception->getCode(), so 404 vs 403 leaks the same thing the message change hides. You'd also end up sending "Form not found" as the body of a 403, which just isn't true.

If you think you've found a real security issue in a Nextcloud project, the right path is HackerOne via the security policy, not a public PR.

I'd lean towards closing this. If you want to tighten the enumeration surface as a hardening change, it'd need to normalize the status code too and come with a test - happy to review that version.

[github-actions]

Hello there,
Thank you so much for taking the time and effort to create a pull request to our Nextcloud project.

We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process.

Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6

Thank you for contributing to Nextcloud and we hope to hear from you soon!

(If you believe you should not receive this message, you can add yourself to the blocklist.)