Skip to content

fix: use correct href for phishing page proceed anyway button #36871

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.

Sign up for GitHub

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

Merged
merged 9 commits into from
Oct 31, 2025
+15 −2
Merged

fix: use correct href for phishing page proceed anyway button #36871

Changes from 2 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
aebf3b0
fix proceed anyway button on c2 domains
mindofmar Oct 14, 2025
e22fdb4
minor refactor
mindofmar Oct 14, 2025
80bac5e
minor fixes
mindofmar Oct 16, 2025
879d2b4
fix tests
mindofmar Oct 17, 2025
593c5dd
fix merge conflicts
mindofmar Oct 27, 2025
fa8fc5c
remove duplicate tracking
mindofmar Oct 27, 2025
f5dc4d9
Merge branch 'main' into fix/phishing-page-proceed-anyway
mindofmar Oct 30, 2025
dd1fcf5
Merge branch 'main' into fix/phishing-page-proceed-anyway
mindofmar Oct 31, 2025
6eebe10
Merge branch 'main' into fix/phishing-page-proceed-anyway
mindofmar Oct 31, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 9 additions & 4 deletions app/scripts/background.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -344,7 +344,7 @@ function maybeDetectPhishing(theController) {

// Determine the block reason based on the type
let blockReason;
let blockedUrl = hostname;
let blockedUrl = href;
if (phishingTestResponse?.result && blockedRequestResponse.result) {
blockReason = `${phishingTestResponse.type} and ${blockedRequestResponse.type}`;
} else if (phishingTestResponse?.result) {
Expand All @@ -354,16 +354,18 @@ function maybeDetectPhishing(theController) {
blockedUrl = details.initiator;
}

const blockedHostname = new URL(blockedUrl).hostname;
Copy link

@cursor cursor bot Oct 14, 2025

Choose a reason for hiding this comment

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

Bug: Phishing Detection Crashes on Invalid URL

The new URL(blockedUrl) constructor can throw a TypeError because blockedUrl might be null, undefined, or the string "null". This happens when blockedUrl is set from details.initiator (e.g., for opaque origins), causing the phishing detection logic to crash instead of processing the blocked request.

Fix in Cursor Fix in Web


if (!isFirefox) {
theController.metaMetricsController.trackEvent(
{
// should we differentiate between background redirection and content script redirection?
event: MetaMetricsEventName.PhishingPageDisplayed,
category: MetaMetricsEventCategory.Phishing,
properties: {
url: blockedUrl,
url: blockedHostname,
referrer: {
url: blockedUrl,
url: blockedHostname,
},
reason: blockReason,
requestDomain: blockedRequestResponse.result
Expand All @@ -376,7 +378,10 @@ function maybeDetectPhishing(theController) {
},
);
}
const querystring = new URLSearchParams({ hostname, href });
const querystring = new URLSearchParams({
hostname: blockedHostname, // used for creating the EPD issue title (false positive report)
href: blockedUrl, // used for displaying the URL on the phsihing warning page + proceed anyway URL
});
const redirectUrl = new URL(phishingPageHref);
redirectUrl.hash = querystring.toString();
const redirectHref = redirectUrl.toString();
Expand Down
Loading