Commit 66d563b8 authored by Daniel Rubery's avatar Daniel Rubery Committed by Commit Bot

Fix crash when analyzing multipart RAR downloads

On Windows, when downloading the second or later part of a multipart
archive, unrar will attempt to open another file. Due to the
modifications we place on unrar to work within the sandbox, we
don't open the file, but instead use File::hOpenFile as the handle.
This is uninitialized memory, which is passed to GetFileType, causing
a crash.

This CL initializes hOpenFile to fix this problem and adds a test that
would have caught the issue.

Fixed: 1135503
Change-Id: I6cc83a5a2b5925900cde7b8b2dc8be64f0620d21
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2490334
Commit-Queue: Varun Khaneja <vakh@chromium.org>
Reviewed-by: default avatarVarun Khaneja <vakh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#820896}
parent 4d621caa
...@@ -137,4 +137,23 @@ IN_PROC_BROWSER_TEST_F(DownloadProtectionServiceBrowserTest, ...@@ -137,4 +137,23 @@ IN_PROC_BROWSER_TEST_F(DownloadProtectionServiceBrowserTest,
EXPECT_EQ("random.exe", requests[0]->archived_binary(0).file_basename()); EXPECT_EQ("random.exe", requests[0]->archived_binary(0).file_basename());
} }
IN_PROC_BROWSER_TEST_F(DownloadProtectionServiceBrowserTest,
MultipartRarInspectionSecondPart) {
embedded_test_server()->ServeFilesFromDirectory(GetTestDataDirectory());
ASSERT_TRUE(embedded_test_server()->Start());
WebUIInfoSingleton::GetInstance()->AddListenerForTesting();
GURL url = embedded_test_server()->GetURL(
"/safe_browsing/rar/multipart.part0002.rar");
DownloadAndWait(url);
const std::vector<std::unique_ptr<ClientDownloadRequest>>& requests =
WebUIInfoSingleton::GetInstance()->client_download_requests_sent();
ASSERT_EQ(1u, requests.size());
ASSERT_EQ(1, requests[0]->archived_binary_size());
EXPECT_EQ("random.exe", requests[0]->archived_binary(0).file_basename());
}
} // namespace safe_browsing } // namespace safe_browsing
...@@ -224,6 +224,15 @@ diff --git b/third_party/unrar/src/file.cpp a/third_party/unrar/src/file.cpp ...@@ -224,6 +224,15 @@ diff --git b/third_party/unrar/src/file.cpp a/third_party/unrar/src/file.cpp
index 52c86c0621af..7dc5b724ca73 100644 index 52c86c0621af..7dc5b724ca73 100644
--- b/third_party/unrar/src/file.cpp --- b/third_party/unrar/src/file.cpp
+++ a/third_party/unrar/src/file.cpp +++ a/third_party/unrar/src/file.cpp
@@ -17,6 +17,10 @@ File::File()
NoSequentialRead=false;
CreateMode=FMF_UNDEFINED;
#endif
+
+#ifdef CHROMIUM_UNRAR
+ hOpenFile=FILE_BAD_HANDLE;
+#endif
}
@@ -51,6 +53,11 @@ bool File::Open(const wchar *Name,uint Mode) @@ -51,6 +53,11 @@ bool File::Open(const wchar *Name,uint Mode)
bool UpdateMode=(Mode & FMF_UPDATE)!=0; bool UpdateMode=(Mode & FMF_UPDATE)!=0;
bool WriteMode=(Mode & FMF_WRITE)!=0; bool WriteMode=(Mode & FMF_WRITE)!=0;
......
...@@ -17,6 +17,10 @@ File::File() ...@@ -17,6 +17,10 @@ File::File()
NoSequentialRead=false; NoSequentialRead=false;
CreateMode=FMF_UNDEFINED; CreateMode=FMF_UNDEFINED;
#endif #endif
#ifdef CHROMIUM_UNRAR
hOpenFile=FILE_BAD_HANDLE;
#endif
} }
......
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