Sync download protection code with Chrome

RESOLVED FIXED in Firefox 51

Status

()

Toolkit
Safe Browsing
P2
normal
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: francois, Assigned: francois)

Tracking

unspecified
mozilla51
Points:
---

Firefox Tracking Flags

(firefox51 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(3 attachments)

(Assignee)

Description

2 years ago
Chrome has updated the Application Reputation protobuf file to make the verdict optional to make it future-proof: https://codereview.chromium.org/2292963004/

They are also removing the .osx file extension: https://bugs.chromium.org/p/chromium/issues/detail?id=641614
(Assignee)

Comment 1

2 years ago
For the rest of the extensions (https://cs.chromium.org/chromium/src/chrome/browser/resources/safe_browsing/download_file_types.asciipb), Chrome only does remote verification for file extensions marked as FULL_PING.

The DangerLevel determines the behavior of the browser when an UNKNOWN verdict is received. We don't currently make use of this information and treat UNKNOWN as SAFE.
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Updated

2 years ago
Assignee: nobody → francois
Priority: -- → P2
(Assignee)

Updated

2 years ago
Status: NEW → ASSIGNED

Comment 5

2 years ago
mozreview-review
Comment on attachment 8790503 [details]
Bug 1302211 - Import latest Safe Browsing CSD file from Chrome.

https://reviewboard.mozilla.org/r/78282/#review77396

rs+
Attachment #8790503 - Flags: review?(gpascutto) → review+

Comment 6

2 years ago
mozreview-review
Comment on attachment 8790504 [details]
Bug 1302211 - Remove .osx extension from download protection.

https://reviewboard.mozilla.org/r/78284/#review77398
Attachment #8790504 - Flags: review?(gpascutto) → review+

Comment 7

2 years ago
mozreview-review
Comment on attachment 8790505 [details]
Bug 1302211 - Sync file extension list with Chrome.

https://reviewboard.mozilla.org/r/78286/#review77402

::: toolkit/components/downloads/ApplicationReputation.cpp:452
(Diff revision 1)
> -    StringEndsWith(fileName, NS_LITERAL_STRING(".dll")) || // Windows
> +    StringEndsWith(fileName, NS_LITERAL_STRING(".dll")) || // Windows executable
>      StringEndsWith(fileName, NS_LITERAL_STRING(".dmg")) || // Mac disk image
>      StringEndsWith(fileName, NS_LITERAL_STRING(".dmgpart")) || // Mac disk image
> -    //StringEndsWith(fileName, NS_LITERAL_STRING(".docb")) ||
> -    StringEndsWith(fileName, NS_LITERAL_STRING(".docm")) || // MS Word
> -    StringEndsWith(fileName, NS_LITERAL_STRING(".docx")) || // MS Word
> +    //StringEndsWith(fileName, NS_LITERAL_STRING(".docb")) || // MS Office
> +    //StringEndsWith(fileName, NS_LITERAL_STRING(".docm")) || // MS Word
> +    //StringEndsWith(fileName, NS_LITERAL_STRING(".docx")) || // MS Word

"Huh"

No more afraid of Office Macros?

Comment 8

2 years ago
mozreview-review
Comment on attachment 8790505 [details]
Bug 1302211 - Sync file extension list with Chrome.

https://reviewboard.mozilla.org/r/78286/#review77406
Attachment #8790505 - Flags: review?(gpascutto) → review+
(Assignee)

Comment 9

2 years ago
(In reply to Gian-Carlo Pascutto [:gcp] from comment #7)
> > -    //StringEndsWith(fileName, NS_LITERAL_STRING(".docb")) ||
> > -    StringEndsWith(fileName, NS_LITERAL_STRING(".docm")) || // MS Word
> > -    StringEndsWith(fileName, NS_LITERAL_STRING(".docx")) || // MS Word
> > +    //StringEndsWith(fileName, NS_LITERAL_STRING(".docb")) || // MS Office
> > +    //StringEndsWith(fileName, NS_LITERAL_STRING(".docm")) || // MS Word
> > +    //StringEndsWith(fileName, NS_LITERAL_STRING(".docx")) || // MS Word
> 
> "Huh"
> 
> No more afraid of Office Macros?

I was also surprised, but Chrome doesn't send them through the remote lookup server:

  # OOXML MS Office files.  These can embed executables, but they don't
  # execute automatically when opened.  These are here to produce UMA metrics.

https://cs.chromium.org/chromium/src/chrome/browser/resources/safe_browsing/download_file_types.asciipb?l=1131

Comment 10

2 years ago
Pushed by fmarier@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/248f9c35af3a
Import latest Safe Browsing CSD file from Chrome. r=gcp
https://hg.mozilla.org/integration/autoland/rev/262d65accc67
Remove .osx extension from download protection. r=gcp
https://hg.mozilla.org/integration/autoland/rev/6795bb0cdda6
Sync file extension list with Chrome. r=gcp

Comment 11

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/248f9c35af3a
https://hg.mozilla.org/mozilla-central/rev/262d65accc67
https://hg.mozilla.org/mozilla-central/rev/6795bb0cdda6
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox51: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
You need to log in before you can comment on or make changes to this bug.