Skip to content

Add support for wildcard characters in allowedUrls#114

Open
Uttkarsh-Srivastava wants to merge 5 commits intofeature/universal_mainfrom
feature/url-whitelisting-ios
Open

Add support for wildcard characters in allowedUrls#114
Uttkarsh-Srivastava wants to merge 5 commits intofeature/universal_mainfrom
feature/url-whitelisting-ios

Conversation

@Uttkarsh-Srivastava
Copy link
Collaborator

No description provided.

@deputydev-agent
Copy link

DeputyDev will no longer review pull requests automatically.To request a review, simply comment #review on your pull request—this will trigger an on-demand review whenever you need it.

@mayankmahavar1mg
Copy link
Collaborator

Summary: Refactors URL whitelist logic in Swift to improve wildcard matching, removes unused file ConfigConstants+URLWhitelist.swift, and fixes config property access bugs.

✅ Strengths

  1. Security improvements
    - Adds proper regex anchoring (^...$) to prevent partial matches (line 219)
    - Implements secure wildcard matching with domain boundary checks
    - Prevents malicious patterns with multiple wildcards in domain patterns
  2. Better pattern matching
    - Separates wildcard vs exact pattern logic clearly
    - Handles IP wildcards (e.g., 192.168.1.) correctly (lines 197-204)
    - Supports subdomain wildcards (
    .example.com and .example.com/)
  3. Code quality
    - Removes duplicate ConfigConstants+URLWhitelist.swift file
    - Consolidates logic into reusable helper methods
    - Better separation of concerns
  4. Bug fixes
    - Line 113: iosConfig.accessControl → WEBVIEW_CONFIG.accessControl (correct scope)
    - Line 113: accessControl.enabled → accessControl.enable (matches config schema)

⚠️ Issues & Concerns

  1. Breaking change potential (lines 196-209)
    - Subdomain wildcard behavior changed: *.example.com now matches example.com itself
    - Previous logic only matched subdomains, not the root domain
    - May break existing configs expecting strict subdomain-only matching

// Now matches BOTH example.com AND sub.example.com
return host.caseInsensitiveCompare(remainder) == .orderedSame ||
host.lowercased().hasSuffix(".(remainder.lowercased())")
2. Inconsistent external URL detection (lines 160-171)
- isExternalUrl() now uses same logic as isUrlAllowed() but with inverted result
- Previously had different logic - could change behavior for edge cases
- Risk: URLs that were considered "external" before may now be internal
3. Missing validation for malformed patterns (line 211)
- Universal wildcard * returns true immediately
- No check if pattern is dangerously permissive
- Should log warning for overly broad patterns
4. Port handling edge case (lines 246-249)
let portMatches = patternPort == nil || urlPort == patternPort ||
(urlPort == nil && ((patternPort == 443 && patternScheme == "https") ||
(patternPort == 80 && patternScheme == "http")))
- Complex logic that's hard to verify correctness
- Consider extracting to separate method with unit tests
5. IP pattern validation weakness (lines 197-204)
- Accepts patterns like 192.168.1.* but doesn't validate it's a valid IP
- Could match non-IP hosts that happen to be numeric
- Consider: Add explicit IP address check

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants