Closed Bug 393305 Opened 13 years ago Closed 12 years ago

Determine multi-scan behavior

Categories

(Toolkit :: Downloads API, defect, P3)

x86
Windows XP
defect

Tracking

()

VERIFIED FIXED
mozilla1.9beta3

People

(Reporter: robarnold, Assigned: jimm)

References

Details

Attachments

(1 file, 2 obsolete files)

Windows Defender registers itself as an av provider but we probably don't want to use it when there is a 'real' av provider available (Norton, McAfee, AVG, etc...). Using all enumerated virus scanners may be excessive.
Not fixing this means the benefit of the new AV scanning patch to Vista users is reduced, because Windows Defender isn't a very good scanner and is always listed first. Maybe we should just try the second scanner if it exists and the first one doesn't find any problems?
Flags: blocking-firefox3?
Blocks: 103487
No longer depends on: 103487
How does IE handle this?
Assignee: nobody → gavin.sharp
Blocking - fine with Gavin's solution in comment 1 as long as it doesn't
introduce significant delay, or just with "use Windows Defender if no other AV
software is listed, otherwise default to the installed AV software"
Flags: blocking-firefox3? → blocking-firefox3+
Attached patch patch (obsolete) — Splinter Review
This implements the second suggestion from Beltzner's comment. It makes a few assumptions, though, that I'm not sure are correct:
1) The Windows defender CLSID won't ever change and is OK to hardcode into the code.
2) Windows Defender's CLSID will always be the first in the enumerator if present.

Rob/Jim/Shawn, do any of you know whether these assumptions are safe to make? Or maybe you have other suggestions for fixing this bug?
Assumption 1 is ok, but I don't think assumption 2 is guaranteed. Enumeration is pretty cheap, so it doesn't hurt to do it once to see if Windows Defender is installed. If Windows Defender is installed, I think we should scan with it, in addition to a real av provider, since it can detect spyware/malware.
No longer blocks: 372972
Target Milestone: --- → Firefox 3 M10
1) The Windows defender CLSID won't ever change and is OK to hardcode into the
code.

I would not count on that. An upgrade might replace it with a new clsid, and unregister the old one.

2) Windows Defender's CLSID will always be the first in the enumerator if
present.

My general testing shows this to be the case, but only on Vista. As rob said though I wouldn't count on it. Unregistering and re-registering the com interface would probably drop it to the bottom of the list.

Also one note - Defender from my understanding is not a virus scanner, it's primarily a spyware scanner. We should definitely scan with it, while also scanning with at least one virus scanner if available. I'd guess few people will be running more than one hard core virus scanner on their system so we should probably just use all available.
Target Milestone: Firefox 3 M10 → Firefox 3 M11
Priority: -- → P5
I did a little testing to understand the order returned. The code we use taps
into a common COM call that iterates COM objects which implement a specific
category. (Categories are a way of associating a group of objects with a common
interface.) On Vista, the iteration is based on the order of the object's clsid
listed in the registry and is returned based on a lowest to highest alpha
numeric order. So the reason why Defender usually shows up first is due to the
fact that it's clsid currently starts with a 2.

If you want to check for the existence of objects that implement the virus
scanning interface, search the reg for the category clsid - 
56FFCC30-D398-11D0-B2AE-00A0C908FA49. The Norton and AV objects should hit on
this within 

HKEY_CLASSES_ROOT\CLSID\(the unique clsid of the object)\Implemented
Categories\56FFCC30-D398-11D0-B2AE-00A0C908FA49

The root object clsid key should have a default string value with the name of
the object.
I'd rather scan with all installed providers than fail to use the "right" one, but this isn't a blocker at this point.
Assignee: gavin.sharp → jmathies
Flags: wanted-firefox3+
Flags: blocking-firefox3-
Flags: blocking-firefox3+
Priority: P5 → P4
Target Milestone: Firefox 3 Mx → Firefox 3 M11
Attached patch multiscan patch v.1 (obsolete) — Splinter Review
adds multiscan support.
One note from testing - afaict, McAfee and Norton have both dropped support for the IOfficeAntiVirus interface in their 2008 products. Both require SP2 now so my guess is they've switched to IAttachmentExecute, or are doing scans some other way. AVG and Defender still support IOfficeAntiVirus.
(In reply to comment #10)
> One note from testing - afaict, McAfee and Norton have both dropped support for
> the IOfficeAntiVirus interface in their 2008 products. Both require SP2 now so
> my guess is they've switched to IAttachmentExecute, or are doing scans some
> other way.

Ugh, that sucks... perhaps we should get a bug filed on addressing that somehow? Can we implement something that uses IAttachmentExecute?
We could hook into it, but it's not like the office api, it's really tapped into the underlying security that came with xp2. I'm not convinced anti-virus software vendors are latched into it either. Plus, a lot of the security built into IAttachmentExecute is already built into firefox, like restrictions on certain file types.

Here's a good support page on how it works:

http://support.microsoft.com/kb/883260

Here are the API's and a good example of how you would check a file:

http://msdn2.microsoft.com/en-us/library/bb776297.aspx

Note the interface is tapped into anti-virus programs as well as IE's internet zone policies.

I'm not entirely sure this is an interface we want ot be hooked up to.

  
In thinking about it, I'd suggest we add the multiscan behavior, then spin out a bug on McAfee and Norton oriented around figuring out what they are doing. I've been reading about IAttachmentExecute, so far I'm not convinced that interface will provide hooks into those anti-virus programs.  
(In reply to comment #13)
> In thinking about it, I'd suggest we add the multiscan behavior, then spin out
> a bug on McAfee and Norton oriented around figuring out what they are doing.
> I've been reading about IAttachmentExecute, so far I'm not convinced that
> interface will provide hooks into those anti-virus programs.  

Yep, sounds good.
(In reply to comment #10)
> One note from testing - afaict, McAfee and Norton have both dropped support for
> the IOfficeAntiVirus interface in their 2008 products. Both require SP2 now so
> my guess is they've switched to IAttachmentExecute, or are doing scans some
> other way. AVG and Defender still support IOfficeAntiVirus.

From running Gavin and Rob Arnold's ScanTest.exe tool against eicar.zip with both Panda and Trend Micro (latest versions), neither one of them appear to implement that IOfficeAntiVirus interface either.
(In reply to comment #11)

> Ugh, that sucks... perhaps we should get a bug filed on addressing that
> somehow? Can we implement something that uses IAttachmentExecute?

I spun off bug 408153 to address that.
Attachment #292479 - Flags: review?(gavin.sharp)
Comment on attachment 292479 [details] [diff] [review]
multiscan patch v.1

sorry, just noticed I need to fix return values on this.
Attachment #292479 - Flags: review?(gavin.sharp)
Attachment #281522 - Attachment is obsolete: true
Attachment #292479 - Attachment is obsolete: true
Attachment #293163 - Flags: review?(gavin.sharp)
Turns out we really need this to fix bug 396553 and friends, so let's make sure we get this ASAP.
Flags: wanted-firefox3+
Flags: blocking-firefox3-
Flags: blocking-firefox3+
Priority: P4 → P3
Duplicate of this bug: 396553
Whiteboard: [has patch][needs review gavin]
Comment on attachment 293163 [details] [diff] [review]
multiscan patch v.2

woot
Attachment #293163 - Flags: review?(gavin.sharp) → review+
Keywords: checkin-needed
Whiteboard: [has patch][needs review gavin] → [has patch][has review][ready to land]
Checking in toolkit/components/downloads/src/nsDownloadScanner.cpp;
/cvsroot/mozilla/toolkit/components/downloads/src/nsDownloadScanner.cpp,v  <--  nsDownloadScanner.cpp
new revision: 1.7; previous revision: 1.6
done
Checking in toolkit/components/downloads/src/nsDownloadScanner.h;
/cvsroot/mozilla/toolkit/components/downloads/src/nsDownloadScanner.h,v  <--  nsDownloadScanner.h
new revision: 1.2; previous revision: 1.1
done
Status: NEW → RESOLVED
Closed: 12 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Whiteboard: [has patch][has review][ready to land]
20080109_0006_firefox-3.0b3pre.en-US.win32.zip
with NIS 2008
bug 396553

seems to be still not scanning.
> seems to be still not scanning.

We split the issue of 2008 products not supporting the interface off in bug 408153.
Flags: in-litmus?
Flags: in-testsuite-
https://litmus.mozilla.org/show_test.cgi?id=5149

in-litmus+
Flags: in-litmus? → in-litmus+
Verified FIXED; this was tested on VM installs where both Windows defender and a third-party vendor's product such as AVG Free or Norton 2007 were installed alongside each other.  The third-party app still scans, indicating our multi-scan support for iOfficeAntiVirus is working.

Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9b3pre) Gecko/2008012404 Minefield/3.0b3pre
Status: RESOLVED → VERIFIED
Product: Firefox → Toolkit
You need to log in before you can comment on or make changes to this bug.