Commit 4a47c3bd authored by Daniel Rubery's avatar Daniel Rubery Committed by Commit Bot

Add timeout to DMG analysis

To limit regression in the total analysis time from Safe Browsing,
cutoff DMG analysis at 10 seconds. Future CLs will institute similar
timeouts for ZIP and RAR archives.

Bug: 948277
Change-Id: Iff7dabfdecc1791d5a83e36dc34a8c57ed99fd9e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1594166Reviewed-by: default avatarNathan Parker <nparker@chromium.org>
Commit-Queue: Daniel Rubery <drubery@chromium.org>
Cr-Commit-Position: refs/heads/master@{#657415}
parent 535a92be
...@@ -14,6 +14,7 @@ ...@@ -14,6 +14,7 @@
#include "base/stl_util.h" #include "base/stl_util.h"
#include "base/strings/string_util.h" #include "base/strings/string_util.h"
#include "base/strings/utf_string_conversions.h" #include "base/strings/utf_string_conversions.h"
#include "base/time/time.h"
#include "chrome/common/safe_browsing/archive_analyzer_results.h" #include "chrome/common/safe_browsing/archive_analyzer_results.h"
#include "chrome/common/safe_browsing/binary_feature_extractor.h" #include "chrome/common/safe_browsing/binary_feature_extractor.h"
#include "chrome/common/safe_browsing/mach_o_image_reader_mac.h" #include "chrome/common/safe_browsing/mach_o_image_reader_mac.h"
...@@ -28,6 +29,9 @@ namespace dmg { ...@@ -28,6 +29,9 @@ namespace dmg {
namespace { namespace {
// The maximum duration of DMG analysis, in milliseconds.
const double kDmgAnalysisTimeoutMs = 10000;
// MachOFeatureExtractor examines files to determine if they are Mach-O, and, // MachOFeatureExtractor examines files to determine if they are Mach-O, and,
// if so, it uses the BinaryFeatureExtractor to obtain information about the // if so, it uses the BinaryFeatureExtractor to obtain information about the
// image. In addition, this class will compute the SHA256 hash of the file. // image. In addition, this class will compute the SHA256 hash of the file.
...@@ -133,6 +137,7 @@ void AnalyzeDMGFile(base::File dmg_file, ArchiveAnalyzerResults* results) { ...@@ -133,6 +137,7 @@ void AnalyzeDMGFile(base::File dmg_file, ArchiveAnalyzerResults* results) {
} }
void AnalyzeDMGFile(DMGIterator* iterator, ArchiveAnalyzerResults* results) { void AnalyzeDMGFile(DMGIterator* iterator, ArchiveAnalyzerResults* results) {
base::Time start_time = base::Time::Now();
results->success = false; results->success = false;
if (!iterator->Open()) if (!iterator->Open())
...@@ -142,10 +147,16 @@ void AnalyzeDMGFile(DMGIterator* iterator, ArchiveAnalyzerResults* results) { ...@@ -142,10 +147,16 @@ void AnalyzeDMGFile(DMGIterator* iterator, ArchiveAnalyzerResults* results) {
results->signature_blob = iterator->GetCodeSignature(); results->signature_blob = iterator->GetCodeSignature();
bool timeout = false;
while (iterator->Next()) { while (iterator->Next()) {
std::unique_ptr<ReadStream> stream = iterator->GetReadStream(); std::unique_ptr<ReadStream> stream = iterator->GetReadStream();
if (!stream) if (!stream)
continue; continue;
if (base::Time::Now() - start_time >=
base::TimeDelta::FromMilliseconds(kDmgAnalysisTimeoutMs)) {
timeout = true;
break;
}
std::string path = base::UTF16ToUTF8(iterator->GetPath()); std::string path = base::UTF16ToUTF8(iterator->GetPath());
...@@ -187,7 +198,8 @@ void AnalyzeDMGFile(DMGIterator* iterator, ArchiveAnalyzerResults* results) { ...@@ -187,7 +198,8 @@ void AnalyzeDMGFile(DMGIterator* iterator, ArchiveAnalyzerResults* results) {
} }
} }
results->success = true; if (!timeout)
results->success = true;
} }
} // namespace dmg } // namespace dmg
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment