Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace insecure self.url?.absoluteString with secure alternative #2067

Conversation

not-a-rootkit
Copy link
Contributor

@not-a-rootkit not-a-rootkit commented Oct 4, 2023

Replace self.url?.absoluteString ?? "" with frame.securityOrigin.host as a more secure fallback value for JSWebAlerts.

Task/Issue URL: https://app.asana.com/0/414709148257752/1205572557120542
Additional Context: https://app.asana.com/0/0/1205594619842512/f

Description:
Our JSWebAlert is showing the wrong origin when falling back on self.url?.absoluteString. This is resulting in JS alerts being generated with the text: Message from originX.com when in fact the message is resulting from JS executing in originY.com.

Steps to test this PR:

  1. Visit https://egghunter.in/ddgpoc/spoof2.html on iOS
  2. Tap on "Click Me"
  3. Notice first alert
  4. See redirect to download-installer.cdn.mozilla...
  5. Cancel/continue the download
  6. Wait a few seconds
  7. Notice popup alert says "egghunter.in" as the host, instead of "mozilla.com" (which is what it said before).

Copy Testing:

  • Use of correct apostrophes in new copy, ie rather than '

Orientation Testing:

  • Portrait
  • Landscape

Device Testing:

  • iPhone SE (1st Gen)
  • iPhone 8
  • iPhone X
  • iPhone 14 Pro
  • iPad

OS Testing:

  • iOS 14
  • iOS 15
  • iOS 16

Theme Testing:

  • Light theme
  • Dark theme

Internal references:

Software Engineering Expectations
Technical Design Template

@samsymons samsymons self-assigned this Oct 21, 2023
@jaceklyp jaceklyp deleted the branch duckduckgo:develop December 6, 2023 15:48
@jaceklyp jaceklyp closed this Dec 6, 2023
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