Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: [FileSystem] Harden against overflows of OperationID a bit better #18572

Merged
merged 3 commits into from Jun 4, 2019
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions patches/common/chromium/.patches
Expand Up @@ -102,3 +102,4 @@ fix_re-entracy_problem_with_invalidateframesinkid.patch
chore_expose_getcontentclient_to_embedders.patch
tabbed_window_lagging.patch
restore_live_region_changed_events_for_processing_by_jaws_focus_mode.patch
filesystem_harden_against_overflows_of_operationid_a_bit_better.patch
@@ -0,0 +1,43 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Marijn Kruisselbrink <mek@chromium.org>
Date: Tue, 29 Jan 2019 19:51:07 +0000
Subject: [FileSystem] Harden against overflows of OperationID a bit better.

Rather than having a UAF when OperationID overflows instead overwrite
the old operation with the new one. Can still cause weirdness, but at
least won't result in UAF. Also update OperationID to uint64_t to
make sure we don't overflow to begin with.

Bug: 925864
Change-Id: Ifdf3fa0935ab5ea8802d91bba39601f02b0dbdc9
Reviewed-on: https://chromium-review.googlesource.com/c/1441498
Commit-Queue: Marijn Kruisselbrink <mek@chromium.org>
Reviewed-by: Victor Costan <pwnall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#627115}

diff --git a/storage/browser/fileapi/file_system_operation_runner.cc b/storage/browser/fileapi/file_system_operation_runner.cc
index fbda72b3cdf851947aa697776e54e0b5092e729b..09af7c0c8c9099489286152009f05ad49d968174 100644
--- a/storage/browser/fileapi/file_system_operation_runner.cc
+++ b/storage/browser/fileapi/file_system_operation_runner.cc
@@ -701,7 +701,7 @@ FileSystemOperationRunner::BeginOperation(
base::WeakPtr<BeginOperationScoper> scope) {
OperationHandle handle;
handle.id = next_operation_id_++;
- operations_.emplace(handle.id, std::move(operation));
+ operations_[handle.id] = std::move(operation);
handle.scope = scope;
return handle;
}
diff --git a/storage/browser/fileapi/file_system_operation_runner.h b/storage/browser/fileapi/file_system_operation_runner.h
index a330f4802d5d5c721d8bba460f25edc2f8e1340a..97f9e0d81163d08644f0cee5b9da21ac24b300af 100644
--- a/storage/browser/fileapi/file_system_operation_runner.h
+++ b/storage/browser/fileapi/file_system_operation_runner.h
@@ -53,7 +53,7 @@ class STORAGE_EXPORT FileSystemOperationRunner
using CopyOrMoveOption = FileSystemOperation::CopyOrMoveOption;
using GetMetadataField = FileSystemOperation::GetMetadataField;

- using OperationID = int;
+ using OperationID = uint64_t;

virtual ~FileSystemOperationRunner();