Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(78)

Side by Side Diff: content/browser/download/quarantine_win.cc

Issue 2025103002: Use better fallback URLs when calling AVScanFile(). (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Some cleanup Created 4 years, 2 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View unified diff | Download patch
OLDNEW
1 // Copyright 2016 The Chromium Authors. All rights reserved. 1 // Copyright 2016 The Chromium Authors. All rights reserved.
2 // Use of this source code is governed by a BSD-style license that can be 2 // Use of this source code is governed by a BSD-style license that can be
3 // found in the LICENSE file. 3 // found in the LICENSE file.
4 4
5 #include "content/browser/download/quarantine.h" 5 #include "content/browser/download/quarantine.h"
6 6
7 #include <windows.h> 7 #include <windows.h>
8 8
9 #include <cguid.h> 9 #include <cguid.h>
10 #include <objbase.h> 10 #include <objbase.h>
11 #include <shellapi.h> 11 #include <shellapi.h>
12 #include <shlobj.h> 12 #include <shlobj.h>
13 #include <shobjidl.h> 13 #include <shobjidl.h>
14 #include <wininet.h>
14 15
15 #include "base/files/file_util.h" 16 #include "base/files/file_util.h"
16 #include "base/guid.h" 17 #include "base/guid.h"
17 #include "base/logging.h" 18 #include "base/logging.h"
18 #include "base/macros.h" 19 #include "base/macros.h"
19 #include "base/metrics/histogram_macros.h" 20 #include "base/metrics/histogram_macros.h"
20 #include "base/metrics/sparse_histogram.h" 21 #include "base/metrics/sparse_histogram.h"
21 #include "base/strings/string_piece.h" 22 #include "base/strings/string_piece.h"
22 #include "base/strings/utf_string_conversions.h" 23 #include "base/strings/utf_string_conversions.h"
23 #include "base/threading/thread_restrictions.h" 24 #include "base/threading/thread_restrictions.h"
(...skipping 140 matching lines...) Expand 10 before | Expand all | Expand 10 after
164 // Typical |save_result| values: 165 // Typical |save_result| values:
165 // S_OK : The file was okay. If any viruses were found, they were cleaned. 166 // S_OK : The file was okay. If any viruses were found, they were cleaned.
166 // E_FAIL : Virus infected. 167 // E_FAIL : Virus infected.
167 // INET_E_SECURITY_PROBLEM : The file was blocked due to security policy. 168 // INET_E_SECURITY_PROBLEM : The file was blocked due to security policy.
168 // 169 //
169 // Any other return value indicates an unexpected error during the scan. 170 // Any other return value indicates an unexpected error during the scan.
170 // 171 //
171 // |full_path| : is the path to the downloaded file. This should be the final 172 // |full_path| : is the path to the downloaded file. This should be the final
172 // path of the download. Must be present. 173 // path of the download. Must be present.
173 // |source_url|: the source URL for the download. If empty, the source will 174 // |source_url|: the source URL for the download. If empty, the source will
174 // not be set. 175 // be set to 'about:internet'.
176 // |referrer_url|: the referrer URL for the download. If empty, the referrer
177 // will not be set.
175 // |client_guid|: the GUID to be set in the IAttachmentExecute client slot. 178 // |client_guid|: the GUID to be set in the IAttachmentExecute client slot.
176 // Used to identify the app to the system AV function. 179 // Used to identify the app to the system AV function.
177 // If GUID_NULL is passed, no client GUID is set. 180 // If GUID_NULL is passed, no client GUID is set.
178 // |save_result|: Receives the result of invoking IAttachmentExecute::Save(). 181 // |save_result|: Receives the result of invoking IAttachmentExecute::Save().
179 bool InvokeAttachmentServices(const base::FilePath& full_path, 182 bool InvokeAttachmentServices(const base::FilePath& full_path,
180 const std::string& source_url, 183 const std::string& source_url,
184 const std::string& referrer_url,
181 const GUID& client_guid, 185 const GUID& client_guid,
182 HRESULT* save_result) { 186 HRESULT* save_result) {
183 base::win::ScopedComPtr<IAttachmentExecute> attachment_services; 187 base::win::ScopedComPtr<IAttachmentExecute> attachment_services;
184 HRESULT hr = attachment_services.CreateInstance(CLSID_AttachmentServices); 188 HRESULT hr = attachment_services.CreateInstance(CLSID_AttachmentServices);
185 *save_result = S_OK; 189 *save_result = S_OK;
186 190
187 if (FAILED(hr)) { 191 if (FAILED(hr)) {
188 // The thread must have COM initialized. 192 // The thread must have COM initialized.
189 DCHECK_NE(CO_E_NOTINITIALIZED, hr); 193 DCHECK_NE(CO_E_NOTINITIALIZED, hr);
190 RecordAttachmentServicesResult( 194 RecordAttachmentServicesResult(
191 AttachmentServicesResult::NO_ATTACHMENT_SERVICES); 195 AttachmentServicesResult::NO_ATTACHMENT_SERVICES);
192 return false; 196 return false;
193 } 197 }
194 198
199 // Note that it is mandatory to check the return values from here on out. If
200 // setting one of the parameters fails, it could leave the object in a state
201 // where the final Save() call will also fail.
202
195 hr = attachment_services->SetClientGuid(client_guid); 203 hr = attachment_services->SetClientGuid(client_guid);
196 if (FAILED(hr)) { 204 if (FAILED(hr)) {
197 RecordAttachmentServicesResult( 205 RecordAttachmentServicesResult(
198 AttachmentServicesResult::FAILED_TO_SET_PARAMETER); 206 AttachmentServicesResult::FAILED_TO_SET_PARAMETER);
199 return false; 207 return false;
200 } 208 }
201 209
202 hr = attachment_services->SetLocalPath(full_path.value().c_str()); 210 hr = attachment_services->SetLocalPath(full_path.value().c_str());
203 if (FAILED(hr)) { 211 if (FAILED(hr)) {
204 RecordAttachmentServicesResult( 212 RecordAttachmentServicesResult(
205 AttachmentServicesResult::FAILED_TO_SET_PARAMETER); 213 AttachmentServicesResult::FAILED_TO_SET_PARAMETER);
206 return false; 214 return false;
207 } 215 }
208 216
209 // Note: SetSource looks like it needs to be called, even if empty. 217 // The source URL could be empty if it was not a valid URL or was not HTTP/S.
210 // Docs say it is optional, but it appears not calling it at all sets 218 // If so, user "about:internet" as a fallback URL. The latter is known to
211 // a zone that is too restrictive. 219 // reliably map to the Internet zone.
212 hr = attachment_services->SetSource(base::UTF8ToWide(source_url).c_str()); 220 //
221 // In addition, URLs that are longer than INTERNET_MAX_URL_LENGTH are also
222 // known to cause problems for URLMon. Hence also use "about:internet" in
223 // these cases. See http://crbug.com/601538.
224 hr = attachment_services->SetSource(
225 source_url.empty() || source_url.size() > INTERNET_MAX_URL_LENGTH
226 ? L"about:internet"
227 : base::UTF8ToWide(source_url).c_str());
213 if (FAILED(hr)) { 228 if (FAILED(hr)) {
214 RecordAttachmentServicesResult( 229 RecordAttachmentServicesResult(
215 AttachmentServicesResult::FAILED_TO_SET_PARAMETER); 230 AttachmentServicesResult::FAILED_TO_SET_PARAMETER);
216 return false; 231 return false;
217 } 232 }
218 233
234 // Only set referrer if one is present and shorter than
235 // INTERNET_MAX_URL_LENGTH. Also, the source_url is authoritative for
236 // determining the relative danger of |full_path| so we don't consider it an
237 // error if we have to skip the |referrer_url|.
238 if (!referrer_url.empty() && referrer_url.size() < INTERNET_MAX_URL_LENGTH) {
239 hr = attachment_services->SetReferrer(
240 base::UTF8ToWide(referrer_url).c_str());
241 if (FAILED(hr)) {
242 RecordAttachmentServicesResult(
243 AttachmentServicesResult::FAILED_TO_SET_PARAMETER);
244 return false;
245 }
246 }
247
219 { 248 {
220 // This method has been known to take longer than 10 seconds in some 249 // This method has been known to take longer than 10 seconds in some
221 // instances. 250 // instances.
222 SCOPED_UMA_HISTOGRAM_LONG_TIMER("Download.AttachmentServices.Duration"); 251 SCOPED_UMA_HISTOGRAM_LONG_TIMER("Download.AttachmentServices.Duration");
223 *save_result = attachment_services->Save(); 252 *save_result = attachment_services->Save();
224 } 253 }
225 RecordAttachmentServicesSaveResult(full_path, *save_result); 254 RecordAttachmentServicesSaveResult(full_path, *save_result);
226 return true; 255 return true;
227 } 256 }
228 257
(...skipping 44 matching lines...) Expand 10 before | Expand all | Expand 10 after
273 } 302 }
274 303
275 if (file_size == 0 || IsEqualGUID(guid, GUID_NULL)) { 304 if (file_size == 0 || IsEqualGUID(guid, GUID_NULL)) {
276 // Calling InvokeAttachmentServices on an empty file can result in the file 305 // Calling InvokeAttachmentServices on an empty file can result in the file
277 // being deleted. Also an anti-virus scan doesn't make a lot of sense to 306 // being deleted. Also an anti-virus scan doesn't make a lot of sense to
278 // perform on an empty file. 307 // perform on an empty file.
279 return SetInternetZoneIdentifierDirectly(file); 308 return SetInternetZoneIdentifierDirectly(file);
280 } 309 }
281 310
282 HRESULT save_result = S_OK; 311 HRESULT save_result = S_OK;
283 bool attachment_services_available = 312 bool attachment_services_available = InvokeAttachmentServices(
284 InvokeAttachmentServices(file, source_url.spec(), guid, &save_result); 313 file, source_url.spec(), referrer_url.spec(), guid, &save_result);
285 if (!attachment_services_available) 314 if (!attachment_services_available)
286 return SetInternetZoneIdentifierDirectly(file); 315 return SetInternetZoneIdentifierDirectly(file);
287 316
288 // If the download file is missing after the call, then treat this as an 317 // If the download file is missing after the call, then treat this as an
289 // interrupted download. 318 // interrupted download.
290 // 319 //
291 // If InvokeAttachmentServices() failed, but the downloaded file is still 320 // If InvokeAttachmentServices() failed, but the downloaded file is still
292 // around, then don't interrupt the download. Attachment Execution Services 321 // around, then don't interrupt the download. Attachment Execution Services
293 // deletes the submitted file if the downloaded file is blocked by policy or 322 // deletes the submitted file if the downloaded file is blocked by policy or
294 // if it was found to be infected. 323 // if it was found to be infected.
295 // 324 //
296 // If the file is still there, then the error could be due to Windows 325 // If the file is still there, then the error could be due to Windows
297 // Attachment Services not being available or some other error during the AES 326 // Attachment Services not being available or some other error during the AES
298 // invocation. In either case, we don't surface the error to the user. 327 // invocation. In either case, we don't surface the error to the user.
299 if (!base::PathExists(file)) 328 if (!base::PathExists(file))
300 return FailedSaveResultToQuarantineResult(save_result); 329 return FailedSaveResultToQuarantineResult(save_result);
301 return QuarantineFileResult::OK; 330 return QuarantineFileResult::OK;
302 } 331 }
303 332
304 } // namespace content 333 } // namespace content
OLDNEW
« no previous file with comments | « content/browser/download/base_file_win_unittest.cc ('k') | content/browser/download/quarantine_win_unittest.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698