malware check non-http URIs

RESOLVED FIXED in mozilla1.9beta2

Status

()

Core
Document Navigation
P1
normal
RESOLVED FIXED
10 years ago
5 years ago

People

(Reporter: dcamp, Assigned: dcamp)

Tracking

(Blocks: 1 bug)

unspecified
mozilla1.9beta2
Points:
---
Bug Flags:
blocking1.9 +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Assignee)

Description

10 years ago
Right now the docshell only malware checks http URIs, because we're not sure other channels properly implement Suspend() right after AsyncOpen().

We should probably:
a) Treat failures from Suspend() as non-fatal
b) Check to make sure the existing channels can be suspended before OnStartRequest().
Flags: blocking1.9?
(Assignee)

Comment 1

10 years ago
Created attachment 280061 [details] [diff] [review]
treat Suspend() failures as nonfatal

This patch restores the old NS_URIChainHasFlags() checks and treats Suspend() failures as nonfatal.
Assignee: nobody → dcamp
Status: NEW → ASSIGNED
Attachment #280061 - Flags: superreview?(cbiesinger)
Attachment #280061 - Flags: review?(bzbarsky)
Comment on attachment 280061 [details] [diff] [review]
treat Suspend() failures as nonfatal

Is there a bug on the nsJSChannel issue?
Attachment #280061 - Flags: review?(bzbarsky) → review+
Flags: blocking1.9? → blocking1.9+

Updated

10 years ago
Priority: -- → P1
Attachment #280061 - Flags: superreview?(cbiesinger) → superreview+

Comment 3

10 years ago
Comment on attachment 280061 [details] [diff] [review]
treat Suspend() failures as nonfatal

Please queue this for landing ASAP. Thanks.
Attachment #280061 - Flags: approval1.9+
(Assignee)

Updated

10 years ago
Keywords: checkin-needed
Checking in docshell/base/nsDocShell.cpp;
/cvsroot/mozilla/docshell/base/nsDocShell.cpp,v  <--  nsDocShell.cpp
new revision: 1.867; previous revision: 1.866
done
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Summary: malware check non-http URIs. → malware check non-http URIs
Target Milestone: --- → mozilla1.9 M10
This patch caused the unit test for bug 377539 to fail, so this patch was backed out. Reopening.

*** 9756 ERROR FAIL | iframe got wrong screen.width | got -1, expected 1280 | /tests/dom/tests/mochitest/bugs/test_bug377539.html
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 6

10 years ago
Created attachment 290259 [details] [diff] [review]
don't classify URIs with no hostname

The patch was triggering a race condition in that unit test - I filed bug 405479 about that race.

That said, we shouldn't be trying to check data: uris at all, or any uri that doesn't have a hostname.  This patch disables malware checking for such uris.
Attachment #290259 - Flags: review?(tony)

Comment 7

10 years ago
Comment on attachment 290259 [details] [diff] [review]
don't classify URIs with no hostname

Looks good, although it's unfortunate that wysiwyg: and data: get a free pass.
Attachment #290259 - Flags: review?(tony) → review+
(Assignee)

Comment 8

10 years ago
Comment on attachment 290259 [details] [diff] [review]
don't classify URIs with no hostname

Checking in src/nsUrlClassifierDBService.cpp;
/cvsroot/mozilla/toolkit/components/url-classifier/src/nsUrlClassifierDBService.cpp,v  <--  nsUrlClassifierDBService.cpp
new revision: 1.38; previous revision: 1.37
done
Checking in src/nsUrlClassifierUtils.cpp;
/cvsroot/mozilla/toolkit/components/url-classifier/src/nsUrlClassifierUtils.cpp,v  <--  nsUrlClassifierUtils.cpp
new revision: 1.9; previous revision: 1.8
done
Checking in tests/unit/test_dbservice.js;
/cvsroot/mozilla/toolkit/components/url-classifier/tests/unit/test_dbservice.js,v  <--  test_dbservice.js
new revision: 1.7; previous revision: 1.6
done
(Assignee)

Comment 9

10 years ago
(In reply to comment #7)
> (From update of attachment 290259 [details] [diff] [review])
> Looks good, although it's unfortunate that wysiwyg: and data: get a free pass.

To clarify, a URI with no hostname will never match the blacklist anyway, because it depends on the value of the hostname.
(Assignee)

Comment 10

10 years ago
Comment on attachment 280061 [details] [diff] [review]
treat Suspend() failures as nonfatal

Checking in nsDocShell.cpp;
/cvsroot/mozilla/docshell/base/nsDocShell.cpp,v  <--  nsDocShell.cpp
new revision: 1.872; previous revision: 1.871
done
(Assignee)

Updated

10 years ago
Status: REOPENED → RESOLVED
Last Resolved: 10 years ago10 years ago
Resolution: --- → FIXED

Updated

5 years ago
Blocks: 812649
You need to log in before you can comment on or make changes to this bug.