Skip to content

[SEC-021][All] Validate intent:// browser_fallback_url scheme#3681

Closed
kristofnemere wants to merge 1 commit intomasterfrom
sec-021-intent-fallback-scheme-validation
Closed

[SEC-021][All] Validate intent:// browser_fallback_url scheme#3681
kristofnemere wants to merge 1 commit intomasterfrom
sec-021-intent-fallback-scheme-validation

Conversation

@kristofnemere
Copy link
Copy Markdown
Contributor

@kristofnemere kristofnemere commented May 5, 2026

Test plan:

refs: SEC-021
affects: Student, Teacher, Parent
release note:

Reject non-http(s) schemes on the browser_fallback_url extra of
intent:// URIs before loading them in CanvasWebView, so a crafted
fallback like javascript: or data: cannot execute in the WebView's
authenticated context.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 5, 2026

🧪 Unit Test Results


📊 Summary

  • Total Tests: 0
  • Failed: 0
  • Skipped: 0
  • Status: ⚠️ No test results found

Last updated: Tue, 05 May 2026 12:57:16 GMT

Copy link
Copy Markdown

@claude claude Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Summary

This is a focused, correct security fix that closes a URL-scheme injection vector in the browser_fallback_url handling path of CanvasWebView.

What the fix does

When an intent: URI cannot be resolved to an installed app, Android's convention allows a browser_fallback_url extra to be loaded instead. Previously that fallback URL was loaded unconditionally, meaning a crafted intent: link could smuggle in a javascript:, file:, or other dangerous scheme through the fallback mechanism. The fix validates the scheme before calling loadUrl.

What's done well

  • Uses Android's Uri.parse() rather than java.net.URI, so malformed fallback URLs won't throw — the scheme simply comes back null and the load is skipped safely.
  • .lowercase() on the scheme ensures the comparison is case-insensitive (e.g., HTTP:// still works).
  • return true is kept outside the inner scheme check, so the navigation event is still consumed even when the fallback URL is rejected. This correctly prevents the rejected URL from falling through to other handlers.
  • The change is minimal and surgical — no unrelated refactoring.

Issues found

  • Silent failure when fallback URL has an unsafe scheme — see inline comment at line 492. When the scheme is blocked, the user gets no feedback and the navigation silently does nothing. A Log.w() would make this much easier to diagnose in the field without any user-visible impact.

val fallbackScheme = Uri.parse(fallbackUrl).scheme?.lowercase()
if (fallbackScheme == "http" || fallbackScheme == "https") {
view.loadUrl(fallbackUrl, extraHeaders)
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When fallbackScheme is neither "http" nor "https", the fallback URL is silently dropped and return true consumes the navigation event — the user gets a broken experience with no indication of what happened. Consider adding a warning log here to aid future debugging:

} else {
    Log.w("CanvasWebView", "Blocked unsafe fallback URL scheme: $fallbackScheme")
}

This doesn't need to be user-visible, but a logcat entry would make it much easier to diagnose reports of broken intent-fallback navigation in the field.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 5, 2026

📊 Code Coverage Report

✅ Student

  • PR Coverage: 42.48%
  • Master Coverage: 42.48%
  • Delta: +0.00%

✅ Teacher

  • PR Coverage: 25.23%
  • Master Coverage: 25.23%
  • Delta: +0.00%

✅ Pandautils

  • PR Coverage: 24.10%
  • Master Coverage: 24.09%
  • Delta: +0.00%

📈 Overall Average

  • PR Coverage: 30.60%
  • Master Coverage: 30.60%
  • Delta: +0.00%

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.

1 participant