fix: require subdomain boundary in isAppwriteOwned domain check#11873
fix: require subdomain boundary in isAppwriteOwned domain check#11873yogeshwaran-c wants to merge 1 commit intoappwrite:1.9.xfrom
Conversation
The isAppwriteOwned method used str_ends_with to check if a domain belongs to Appwrite. This incorrectly matched domains like evil-appwrite.network against appwrite.network because str_ends_with only checks the suffix without requiring a subdomain boundary (dot). This could allow a non-Appwrite domain to bypass DNS verification and be marked as verified with owner Appwrite, since isAppwriteOwned sets RULE_STATUS_VERIFIED for matching domains. The fix requires either an exact match or a proper subdomain match (preceded by a dot separator).
Greptile SummaryThis PR fixes a domain confusion vulnerability in Confidence Score: 5/5Safe to merge — single-line fix correctly closes a domain verification bypass with no regressions. The change is minimal, targeted, and logically correct. The dot-boundary pattern is the standard approach for subdomain matching. All four callers of No files require special attention. Important Files Changed
Reviews (1): Last reviewed commit: "fix: require subdomain boundary in isApp..." | Re-trigger Greptile |
What kind of change does this PR introduce?
Bug fix
What is the current behavior?
The
isAppwriteOwned()method insrc/Appwrite/Platform/Modules/Proxy/Action.phpuses\str_ends_with($domain, $appwriteDomain)to determine if a domain is owned by Appwrite. This check is too permissive — it matches any domain whose name ends with the Appwrite domain string, even if it is not an actual subdomain.For example, if
_APP_DOMAIN_SITESis set toappwrite.network:myapp.appwrite.network→ correctly matches (legitimate subdomain)evil-appwrite.network→ incorrectly matches (completely different domain)When
isAppwriteOwned()returnstrue, the proxy rule creation endpoints skip DNS verification and immediately set the rule status toRULE_STATUS_VERIFIEDwith owner'Appwrite'. This means a domain likeevil-appwrite.networkwould bypass domain verification entirely.What is the new behavior?
The check now requires either:
$domain === $appwriteDomain), or'.' . $appwriteDomain(note the dot separator)This ensures only legitimate subdomains of Appwrite-owned domains are recognized:
myapp.appwrite.network→ matches (ends with.appwrite.network)evil-appwrite.network→ does not match (does not end with.appwrite.network)Additional context
The fix is a single-line change. The same pattern of requiring a dot boundary for subdomain matching is a well-established security practice to prevent domain confusion attacks.
Test plan
test.appwrite.networkwhen_APP_DOMAIN_SITES=appwrite.network) — should be recognized as Appwrite-owned and skip verificationevil-appwrite.network) — should NOT be recognized as Appwrite-owned and should require DNS verificationappwrite.network) — should be recognized as Appwrite-owned