1053645 - Downloads blocked by Application Reputation are retried on a restart

Status: RESOLVED WORKSFORME

(Toolkit :: Downloads API, defect)

Description

[Gian-Carlo Pascutto [:gcp]]

After trying the STR in Bug 1048508, I noticed that when you restart the browser, we retry the download automatically, i.e. if we block a download because we think it's malware, we will just try to download it again on every next restart.

Comment 1

[[:mmc] Monica Chew (no longer reading bugmail)]

That means something is going wrong in both the cleanup and retry code, or else the download state is not getting set correctly to DOWNLOAD_DIRTY.

http://mxr.mozilla.org/mozilla-central/source/toolkit/components/downloads/nsDownloadManager.cpp#1845

 nsresult
1839 nsDownloadManager::RetryDownload(nsDownload* dl)
1840 {
1841   // if our download is not canceled or failed, we should fail
1842   if (dl->mDownloadState != nsIDownloadManager::DOWNLOAD_FAILED &&
1843       dl->mDownloadState != nsIDownloadManager::DOWNLOAD_BLOCKED_PARENTAL &&
1844       dl->mDownloadState != nsIDownloadManager::DOWNLOAD_BLOCKED_POLICY &&
1845       dl->mDownloadState != nsIDownloadManager::DOWNLOAD_DIRTY &&
1846       dl->mDownloadState != nsIDownloadManager::DOWNLOAD_CANCELED)
1847     return NS_ERROR_FAILURE;
1848 

nsresult
2067 nsDownloadManager::CleanUp(mozIStorageConnection* aDBConn)
2068 {
2069   DownloadState states[] = { nsIDownloadManager::DOWNLOAD_FINISHED,
2070                              nsIDownloadManager::DOWNLOAD_FAILED,
2071                              nsIDownloadManager::DOWNLOAD_CANCELED,
2072                              nsIDownloadManager::DOWNLOAD_BLOCKED_PARENTAL,
2073                              nsIDownloadManager::DOWNLOAD_BLOCKED_POLICY,
2074                              nsIDownloadManager::DOWNLOAD_DIRTY };
2075 
2076   nsCOMPtr<mozIStorageStatement> stmt;
2077   nsresult rv = aDBConn->CreateStatement(NS_LITERAL_CSTRING(
2078     "DELETE FROM moz_downloads "
2079     "WHERE state = ? "
2080       "OR state = ? "
2081       "OR state = ? "
2082       "OR state = ? "
2083       "OR state = ? "
2084       "OR state = ?"), getter_AddRefs(stmt));

Comment 2

[[:mmc] Monica Chew (no longer reading bugmail)]

Paolo, is toolkit/components/downloads/nsDownloadManager.cpp in use any more, or has it been replaced entirely by the modules in toolkit/components/jsdownloads?

Comment 3

[:Paolo Amadini]

(In reply to [:mmc] Monica Chew (please use needinfo) from comment #2)
> Paolo, is toolkit/components/downloads/nsDownloadManager.cpp in use any
> more, or has it been replaced entirely by the modules in
> toolkit/components/jsdownloads?

In Firefox for Desktop, we don't use the code from nsDownloadManager.cpp for download management anymore, we use only some methods for retrieving the download directories.

Do we have a persistent URL for testing Application Reputation blocking?

Comment 4

[[:mmc] Monica Chew (no longer reading bugmail)]

Well, that explains it then! :) We don't really have a persistent URL, maybe gcp knows one. The unittests use synthetic lists or a dummy server. I normally modifying shouldBlock=true (terrible hack).

Comment 5

[Gian-Carlo Pascutto [:gcp]]

I don't know of any way to test the download protection or verify that it works wrt updates etc. We should probably ask the engineer who wrote it to fix this, or perhaps ask Google if there's any known URLs guaranteed to be on the local/remote blocklists.

Comment 6

[Mike]

Here are two that are showing up on my end.
Blocked on a Windows 8.1 system: http://www.oxid.it/downloads/ca_setup.exe
Blocked on a Windows XP system: http://www.lcpsoft.com/download/lcp504en.exe

Comment 7

[[:mmc] Monica Chew (no longer reading bugmail)]

Attachment: dm-retry

I'm unable to reproduce with this patch or with comment 6. I'm getting the warning but no retries on restart.

Comment 8

[[:mmc] Monica Chew (no longer reading bugmail)]

http://hg.mozilla.org/mozilla-central/rev/a818c7fa1372 was the state change integration.

Comment 9

[Mike]

To add more context, I am on the Beta upgrades, so I am currently at Firefox 32.0 Beta 8 and this started approximately Firefox 32.0 Beta 2 on the XP. It may have gone back further than that version, but I may not have had something blocked until then. I am pretty sure it was around the time Firefox started to attempt to report crashes and recover pages when it was legitimately closed, though it may have been just before that time also, but not long.

Comment 10

[Gian-Carlo Pascutto [:gcp]]

Reproducible on current Nightly:

1) Go to https://www.dropbox.com/sh/0d86al68t1j3ed8/AAAi7qReKAQVYy6md0T44a9Fa
2) Go to Download->as zip
3) Download fails
4) Is restarted upon restarting the browser

Comment 11

[Gian-Carlo Pascutto [:gcp]]

Here's 2 more links to test this:

http://www.nirsoft.net/utils/webbrowserpassview.zip
http://downloadpack.net/skinpack/win9/SkinPack_Windows9_V3.0.zip

Comment 12

[:Paolo Amadini]

This may have been fixed by the new interface to unblock downloads. I tried with the malware link at <https://testsafebrowsing.appspot.com/> and the issue does not seem to happen. Conversely, we don't persist the download information, so the opposite happens and we forget about the placeholder file, as indicated in bug 1139914. I'm closing this bug, feel free to reopen if the issue still happens to you.