Inspect the file contents within RAR files |
|
Issue descriptionCurrently we only ping SB with the file names within the RAR. We should extract the files so that we can send lengths and digests as well.
,
Nov 30
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/aa21b3f577d2d912e7d3898af5ee42e01a00cacd commit aa21b3f577d2d912e7d3898af5ee42e01a00cacd Author: Daniel Rubery <drubery@chromium.org> Date: Fri Nov 30 21:44:20 2018 Add util method to update ArchiveAnalyzerResults for a single file This CL breaks out the code that inspects an individual file within a ZIP archive. This will be shared between the ZIP and RAR inspection, when RAR files begin doing content inspection. Bug: 909778 Change-Id: I7acf1cabd472f112f2ed7c31735688cae7a6d122 Reviewed-on: https://chromium-review.googlesource.com/c/1354103 Commit-Queue: Daniel Rubery <drubery@chromium.org> Reviewed-by: Brian White <bcwhite@chromium.org> Reviewed-by: Jay Civelli <jcivelli@chromium.org> Reviewed-by: Varun Khaneja <vakh@chromium.org> Cr-Commit-Position: refs/heads/master@{#612799} [modify] https://crrev.com/aa21b3f577d2d912e7d3898af5ee42e01a00cacd/chrome/common/safe_browsing/BUILD.gn [modify] https://crrev.com/aa21b3f577d2d912e7d3898af5ee42e01a00cacd/chrome/common/safe_browsing/archive_analyzer_results.cc [modify] https://crrev.com/aa21b3f577d2d912e7d3898af5ee42e01a00cacd/chrome/common/safe_browsing/archive_analyzer_results.h [modify] https://crrev.com/aa21b3f577d2d912e7d3898af5ee42e01a00cacd/chrome/common/safe_browsing/zip_analyzer.cc [modify] https://crrev.com/aa21b3f577d2d912e7d3898af5ee42e01a00cacd/chrome/services/file_util/public/cpp/sandboxed_zip_analyzer_unittest.cc [modify] https://crrev.com/aa21b3f577d2d912e7d3898af5ee42e01a00cacd/chrome/utility/safe_browsing/mac/BUILD.gn [modify] https://crrev.com/aa21b3f577d2d912e7d3898af5ee42e01a00cacd/tools/metrics/histograms/histograms.xml
,
Dec 3
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/98e53045947734df30e2634b5e38250350de186c commit 98e53045947734df30e2634b5e38250350de186c Author: Guido Urdaneta <guidou@chromium.org> Date: Mon Dec 03 13:45:14 2018 Revert "Add util method to update ArchiveAnalyzerResults for a single file" This reverts commit aa21b3f577d2d912e7d3898af5ee42e01a00cacd. Reason for revert: Suspect of introducing consistent failure on Mac ASAN 64 tests. First failed run: https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Mac%20ASan%2064%20Tests%20%281%29/46988 Sample logs: unit_tests Run on OS: 'Mac-10.13' Shard duration: 0:07:56.378119 failures: FileAnalyzerTest.ArchivedArchiveSetForZipNoArchive FileAnalyzerTest.ArchivedExecutableFalseForZipNoExecutable DownloadProtectionServiceTest.CheckClientDownloadZip SandboxedZipAnalyzerTest.ZippedAppWithUnsignedAndSignedExecutable SandboxedZipAnalyzerTest.NoBinaries FileAnalyzerTest.ArchivedBinariesSkipsSafeFiles ==34531==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x700015467c14 at pc 0x00013e99c44d bp 0x700015467950 sp 0x7000154670f8 READ of size 5 at 0x700015467c14 thread T16 #0 0x13e99c44c in __asan_after_dynamic_init ??:0:0 #1 0x10e3ac931 in safe_browsing::UpdateArchiveAnalyzerResultsWithFile(base::FilePath, base::File*, bool, safe_browsing::ArchiveAnalyzerResults*) ??:0:0 #2 0x10e3a6707 in safe_browsing::zip_analyzer::AnalyzeZipFile(base::File, base::File, safe_browsing::ArchiveAnalyzerResults*) ??:0:0 #3 0x115236586 in SafeArchiveAnalyzer::AnalyzeZipFile(base::File, base::File, base::OnceCallback<void (safe_browsing::ArchiveAnalyzerResults const&)>) ??:0:0 #4 0x10bc0b332 in chrome::mojom::SafeArchiveAnalyzerStubDispatch::AcceptWithResponder(chrome::mojom::SafeArchiveAnalyzer*, mojo::Message*, std::__1::unique_ptr<mojo::MessageReceiverWithStatus, std::__1::default_delete<mojo::MessageReceiverWithStatus> >) ??:0:0 #5 0x115234a40 in chrome::mojom::SafeArchiveAnalyzerStub<mojo::RawPtrImplRefTraits<chrome::mojom::SafeArchiveAnalyzer> >::AcceptWithResponder(mojo::Message*, std::__1::unique_ptr<mojo::MessageReceiverWithStatus, std::__1::default_delete<mojo::MessageReceiverWithStatus> >) ??:0:0 #6 0x1165eeb32 in mojo::InterfaceEndpointClient::HandleValidatedMessage(mojo::Message*) ??:0:0 #7 0x1165ed1e0 in mojo::FilterChain::Accept(mojo::Message*) ??:0:0 #8 0x1165f278b in mojo::InterfaceEndpointClient::HandleIncomingMessage(mojo::Message*) ??:0:0 #9 0x116605686 in mojo::internal::MultiplexRouter::ProcessIncomingMessage(mojo::internal::MultiplexRouter::MessageWrapper*, mojo::internal::MultiplexRouter::ClientCallBehavior, base::SequencedTaskRunner*) ??:0:0 #10 0x11660372b in mojo::internal::MultiplexRouter::Accept(mojo::Message*) ??:0:0 #11 0x1165ed1e0 in mojo::FilterChain::Accept(mojo::Message*) ??:0:0 #12 0x1165dcea8 in mojo::Connector::ReadSingleMessage(unsigned int*) ??:0:0 #13 0x1165df32a in mojo::Connector::ReadAllAvailableMessages() ??:0:0 #14 0x1165deda1 in mojo::Connector::OnHandleReadyInternal(unsigned int) ??:0:0 #15 0x10d429d44 in mojo::SimpleWatcher::DiscardReadyState(base::RepeatingCallback<void (unsigned int)> const&, unsigned int, mojo::HandleSignalsState const&) ??:0:0 #16 0x11a802cf5 in mojo::SimpleWatcher::OnHandleReady(int, unsigned int, mojo::HandleSignalsState const&) ??:0:0 #17 0x11a803cbd in void base::internal::Invoker<base::internal::BindState<void (mojo::SimpleWatcher::*)(int, unsigned int, mojo::HandleSignalsState const&), base::WeakPtr<mojo::SimpleWatcher>, int, unsigned int, mojo::HandleSignalsState>, void ()>::RunImpl<void (mojo::SimpleWatcher::* const&)(int, unsigned int, mojo::HandleSignalsState const&), std::__1::tuple<base::WeakPtr<mojo::SimpleWatcher>, int, unsigned int, mojo::HandleSignalsState> const&, 0ul, 1ul, 2ul, 3ul>(void (mojo::SimpleWatcher::* const&)(int, unsigned int, mojo::HandleSignalsState const&), std::__1::tuple<base::WeakPtr<mojo::SimpleWatcher>, int, unsigned int, mojo::HandleSignalsState> const&, std::__1::integer_sequence<unsigned long, 0ul, 1ul, 2ul, 3ul>) ??:0:0 #18 0x11890131c in base::debug::TaskAnnotator::RunTask(char const*, base::PendingTask*) ??:0:0 #19 0x118995271 in base::MessageLoopImpl::RunTask(base::PendingTask*) ??:0:0 #20 0x11899691f in base::MessageLoopImpl::DoWork() ??:0:0 #21 0x118999eb3 in base::MessagePumpDefault::Run(base::MessagePump::Delegate*) ??:0:0 #22 0x118994259 in base::MessageLoopImpl::Run(bool) ??:0:0 #23 0x118a49a1c in base::RunLoop::Run() ??:0:0 #24 0x118b962e2 in base::Thread::Run(base::RunLoop*) ??:0:0 #25 0x118b96e9d in base::Thread::ThreadMain() ??:0:0 #26 0x118c72a0d in base::(anonymous namespace)::ThreadFunc(void*) ??:0:0 #27 0x7fff74564660 in _pthread_body ??:0:0 #28 0x7fff7456450c in _pthread_start ??:0:0 Original change's description: > Add util method to update ArchiveAnalyzerResults for a single file > > This CL breaks out the code that inspects an individual file within a > ZIP archive. This will be shared between the ZIP and RAR inspection, > when RAR files begin doing content inspection. > > Bug: 909778 > Change-Id: I7acf1cabd472f112f2ed7c31735688cae7a6d122 > Reviewed-on: https://chromium-review.googlesource.com/c/1354103 > Commit-Queue: Daniel Rubery <drubery@chromium.org> > Reviewed-by: Brian White <bcwhite@chromium.org> > Reviewed-by: Jay Civelli <jcivelli@chromium.org> > Reviewed-by: Varun Khaneja <vakh@chromium.org> > Cr-Commit-Position: refs/heads/master@{#612799} TBR=jcivelli@chromium.org,bcwhite@chromium.org,vakh@chromium.org,drubery@chromium.org # Not skipping CQ checks because original CL landed > 1 day ago. Bug: 909778 Change-Id: I92e193707da4a28cee94aa1277a6ca567650760f Reviewed-on: https://chromium-review.googlesource.com/c/1358491 Reviewed-by: Guido Urdaneta <guidou@chromium.org> Commit-Queue: Guido Urdaneta <guidou@chromium.org> Cr-Commit-Position: refs/heads/master@{#613073} [modify] https://crrev.com/98e53045947734df30e2634b5e38250350de186c/chrome/common/safe_browsing/BUILD.gn [modify] https://crrev.com/98e53045947734df30e2634b5e38250350de186c/chrome/common/safe_browsing/archive_analyzer_results.cc [modify] https://crrev.com/98e53045947734df30e2634b5e38250350de186c/chrome/common/safe_browsing/archive_analyzer_results.h [modify] https://crrev.com/98e53045947734df30e2634b5e38250350de186c/chrome/common/safe_browsing/zip_analyzer.cc [modify] https://crrev.com/98e53045947734df30e2634b5e38250350de186c/chrome/services/file_util/public/cpp/sandboxed_zip_analyzer_unittest.cc [modify] https://crrev.com/98e53045947734df30e2634b5e38250350de186c/chrome/utility/safe_browsing/mac/BUILD.gn [modify] https://crrev.com/98e53045947734df30e2634b5e38250350de186c/tools/metrics/histograms/histograms.xml
,
Dec 7
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/2c065bb11d7a6e0227c0d32e043e458e1ef76091 commit 2c065bb11d7a6e0227c0d32e043e458e1ef76091 Author: Daniel Rubery <drubery@chromium.org> Date: Thu Dec 06 20:57:26 2018 Reland: Add util method to update ArchiveAnalyzerResults for a single file This CL breaks out the code that inspects an individual file within a ZIP archive. This will be shared between the ZIP and RAR inspection, when RAR files begin doing content inspection. This is a reland of https://chromium-review.googlesource.com/c/chromium/src/+/1354103 The OOB read has been fixed between patchsets 1 and 2. Bug: 909778 Change-Id: I707364516217136ee95174bbae930b5df0cf54ed TBR: jcivelli@ Reviewed-on: https://chromium-review.googlesource.com/c/1359037 Commit-Queue: Daniel Rubery <drubery@chromium.org> Reviewed-by: Varun Khaneja <vakh@chromium.org> Reviewed-by: Brian White <bcwhite@chromium.org> Cr-Commit-Position: refs/heads/master@{#614486} [modify] https://crrev.com/2c065bb11d7a6e0227c0d32e043e458e1ef76091/chrome/common/safe_browsing/BUILD.gn [modify] https://crrev.com/2c065bb11d7a6e0227c0d32e043e458e1ef76091/chrome/common/safe_browsing/archive_analyzer_results.cc [modify] https://crrev.com/2c065bb11d7a6e0227c0d32e043e458e1ef76091/chrome/common/safe_browsing/archive_analyzer_results.h [modify] https://crrev.com/2c065bb11d7a6e0227c0d32e043e458e1ef76091/chrome/common/safe_browsing/zip_analyzer.cc [modify] https://crrev.com/2c065bb11d7a6e0227c0d32e043e458e1ef76091/chrome/services/file_util/public/cpp/sandboxed_zip_analyzer_unittest.cc [modify] https://crrev.com/2c065bb11d7a6e0227c0d32e043e458e1ef76091/chrome/utility/safe_browsing/mac/BUILD.gn [modify] https://crrev.com/2c065bb11d7a6e0227c0d32e043e458e1ef76091/tools/metrics/histograms/histograms.xml
,
Dec 10
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ec3682e3c9061c10f26ea9e5cdcf3c53f3f74387 commit ec3682e3c9061c10f26ea9e5cdcf3c53f3f74387 Author: Daniel Rubery <drubery@chromium.org> Date: Mon Dec 10 22:47:32 2018 Extract the contents of RAR files and report hashes and signatures Bug: 909778 Change-Id: Id1f5cb828b8ad5cc13dcff571e026dd69b3fbc76 Reviewed-on: https://chromium-review.googlesource.com/c/1327238 Reviewed-by: Robert Sesek <rsesek@chromium.org> Reviewed-by: Ken Rockot <rockot@google.com> Reviewed-by: Varun Khaneja <vakh@chromium.org> Commit-Queue: Daniel Rubery <drubery@chromium.org> Cr-Commit-Position: refs/heads/master@{#615291} [modify] https://crrev.com/ec3682e3c9061c10f26ea9e5cdcf3c53f3f74387/chrome/common/safe_browsing/BUILD.gn [modify] https://crrev.com/ec3682e3c9061c10f26ea9e5cdcf3c53f3f74387/chrome/common/safe_browsing/rar_analyzer.cc [modify] https://crrev.com/ec3682e3c9061c10f26ea9e5cdcf3c53f3f74387/chrome/common/safe_browsing/rar_analyzer.h [modify] https://crrev.com/ec3682e3c9061c10f26ea9e5cdcf3c53f3f74387/chrome/services/file_util/DEPS [modify] https://crrev.com/ec3682e3c9061c10f26ea9e5cdcf3c53f3f74387/chrome/services/file_util/public/cpp/sandboxed_rar_analyzer.cc [modify] https://crrev.com/ec3682e3c9061c10f26ea9e5cdcf3c53f3f74387/chrome/services/file_util/public/cpp/sandboxed_rar_analyzer.h [modify] https://crrev.com/ec3682e3c9061c10f26ea9e5cdcf3c53f3f74387/chrome/services/file_util/public/cpp/sandboxed_rar_analyzer_unittest.cc [modify] https://crrev.com/ec3682e3c9061c10f26ea9e5cdcf3c53f3f74387/chrome/services/file_util/public/mojom/safe_archive_analyzer.mojom [modify] https://crrev.com/ec3682e3c9061c10f26ea9e5cdcf3c53f3f74387/chrome/services/file_util/safe_archive_analyzer.cc [modify] https://crrev.com/ec3682e3c9061c10f26ea9e5cdcf3c53f3f74387/chrome/services/file_util/safe_archive_analyzer.h [modify] https://crrev.com/ec3682e3c9061c10f26ea9e5cdcf3c53f3f74387/components/safe_browsing/features.cc [modify] https://crrev.com/ec3682e3c9061c10f26ea9e5cdcf3c53f3f74387/components/safe_browsing/features.h [modify] https://crrev.com/ec3682e3c9061c10f26ea9e5cdcf3c53f3f74387/third_party/unrar/README.chromium [modify] https://crrev.com/ec3682e3c9061c10f26ea9e5cdcf3c53f3f74387/third_party/unrar/patches/5.6.8.patch [modify] https://crrev.com/ec3682e3c9061c10f26ea9e5cdcf3c53f3f74387/third_party/unrar/src/archive.cpp [modify] https://crrev.com/ec3682e3c9061c10f26ea9e5cdcf3c53f3f74387/third_party/unrar/src/archive.hpp [modify] https://crrev.com/ec3682e3c9061c10f26ea9e5cdcf3c53f3f74387/third_party/unrar/src/extract.cpp [modify] https://crrev.com/ec3682e3c9061c10f26ea9e5cdcf3c53f3f74387/third_party/unrar/src/file.cpp [modify] https://crrev.com/ec3682e3c9061c10f26ea9e5cdcf3c53f3f74387/third_party/unrar/src/unrar_wrapper.h
,
Jan 4
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/2c3b194535c675bb42b060deae3cf79e06f2725a commit 2c3b194535c675bb42b060deae3cf79e06f2725a Author: Daniel Rubery <drubery@chromium.org> Date: Fri Jan 04 19:16:52 2019 Define NOVOLUME for unrar code To prevent attempting to extract other volumes within the sandbox, this CL puts unrar in NOVOLUME mode. I also added a quick test to ensure we still detect the presence of files in NOVOLUME mode, so this should be safe. Note: The generation of multipart rar files seems a little finicky. The following two commands worked for me. dd if=/dev/urandom of=random.exe bs=1k count=1 rar a -v1b multipart.rar random.exe Bug: 917679, 909778 Change-Id: I472b0e959d7538dc9136597ada631707b044e765 Reviewed-on: https://chromium-review.googlesource.com/c/1393803 Reviewed-by: Jay Civelli <jcivelli@chromium.org> Reviewed-by: Varun Khaneja <vakh@chromium.org> Commit-Queue: Daniel Rubery <drubery@chromium.org> Cr-Commit-Position: refs/heads/master@{#620026} [modify] https://crrev.com/2c3b194535c675bb42b060deae3cf79e06f2725a/chrome/services/file_util/public/cpp/sandboxed_rar_analyzer_unittest.cc [add] https://crrev.com/2c3b194535c675bb42b060deae3cf79e06f2725a/chrome/test/data/safe_browsing/rar/multipart.part0001.rar [add] https://crrev.com/2c3b194535c675bb42b060deae3cf79e06f2725a/chrome/test/data/safe_browsing/rar/multipart.part0002.rar [modify] https://crrev.com/2c3b194535c675bb42b060deae3cf79e06f2725a/third_party/unrar/BUILD.gn
,
Jan 17
(5 days ago)
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/541e742e2d500e85328013b1d3524e71596277da commit 541e742e2d500e85328013b1d3524e71596277da Author: Daniel Rubery <drubery@chromium.org> Date: Thu Jan 17 19:16:28 2019 Add maximum inspection size for RAR archives Finch showed a significant difference in inspection time when enabling content inspection for RAR archives (http://shortn/_64qaXWf78L). I expect this is because RARs archives tend to be very large, so unpacking the archive takes a lot of time. This CL has the RAR analyzer fall back to the old behavior for large (> 50MB) archives. Bug: 909778 Change-Id: I5a42c4921d7858cdd8e68d629b6326c450b41d75 Reviewed-on: https://chromium-review.googlesource.com/c/1415253 Commit-Queue: Daniel Rubery <drubery@chromium.org> Reviewed-by: Varun Khaneja <vakh@chromium.org> Cr-Commit-Position: refs/heads/master@{#623787} [modify] https://crrev.com/541e742e2d500e85328013b1d3524e71596277da/chrome/browser/resources/safe_browsing/download_file_types.asciipb [modify] https://crrev.com/541e742e2d500e85328013b1d3524e71596277da/chrome/browser/safe_browsing/download_protection/file_analyzer_unittest.cc [modify] https://crrev.com/541e742e2d500e85328013b1d3524e71596277da/chrome/common/safe_browsing/rar_analyzer.cc |
|
►
Sign in to add a comment |
|
Comment 1 by vakh@chromium.org
, Nov 28