fix(security): resolve profile update validation bypass and SSRF#507
Open
prince-shakyaa wants to merge 3 commits into
Open
fix(security): resolve profile update validation bypass and SSRF#507prince-shakyaa wants to merge 3 commits into
prince-shakyaa wants to merge 3 commits into
Conversation
17c86b3 to
40740ff
Compare
Author
|
Hi @saikishu @e2hln , |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
fix(security): resolve profile update validation bypass and SSRF via insecure avatar URLs
Description
Fixes #506
This Pull Request resolves a high-severity security vulnerability in the user profile and sharing module.
The Problem
Previously, the
ProfileUpdateRequestallowed updating fields optionally. The route only validated that theavatar_urlstarted withhttps://ifavatar_typewas explicitly set to"url"in the current request payload:If a user's profile already had
avatar_type = "url"in the database, they could bypass this security check by sending a PUT request containing onlyavatar_url(omittingavatar_typeentirely). The backend successfully saved the insecure URL (e.g.http://127.0.0.1:8500/orhttp://169.254.169.254/).When generating the profile sharing card (
/share/profile/{username}/card.png), the backend issued an asynchronoushttpx.getrequest directly to this user-supplied insecure URL to fetch and base64-encode the image. This resulted in a Server-Side Request Forgery (SSRF) vulnerability, enabling attackers to scan local ports, query intranet systems, or access cloud metadata services.Furthermore, even if
https://was enforced, an attacker could usehttps://127.0.0.1or set up a public server that redirects (302 Found) back to a local IP address.The Fix
This PR introduces a robust defense-in-depth mitigation:
update_profileroute validation now computes theeffective_avatar_typeandeffective_avatar_url(taking the incoming request's type, or falling back to the existing profile's saved type in the database if omitted).https://, the effective URL is passed tois_ssrf_safe. This resolves the hostname and blocks any IP in local, link-local, or private RFC-1918 subnets.httpx.AsyncClientthat fetches the avatar inshare.pyis now configured withfollow_redirects=False.Key Changes
finbot/apps/ctf/routes/profile.pyis_ssrf_safeusingsocketandipaddressto strictly block private subnets.finbot/apps/ctf/routes/share.pyfollow_redirects=Falseto neutralize redirect-based SSRF.Verification & Testing
Manual Verification
url.avatar_typewhile settingavatar_urltohttp://127.0.0.1. Blocked (400 Bad Request).https://127.0.0.1orhttps://169.254.169.254. Blocked (400 Bad Request).httpxrefuses to follow the redirect.