From 7f6d9e9c7ffa756c2a03ea97aff910abab1cae71 Mon Sep 17 00:00:00 2001 From: Rupert Ben Wiser Date: Fri, 29 Sep 2023 15:54:13 +0000 Subject: [PATCH] Add support for matching with key allowlist WebView makes use of this allowlist. We are hoping to include switches and features in our crash keys as users can enable these with an easily available developer UI. These crash keys follow a pattern of "switch-" so it is impractical to indefinitely add a larger list of switch keys. Adding this matcher lets us rather add "switch-*". Bug: 1484644 Change-Id: I667cef70cce1efb0710b4a2f009d8d80a1eeae5a Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/4894239 Commit-Queue: Rupert Wiser Reviewed-by: Joshua Peraza --- DEPS | 2 +- snapshot/sanitized/module_snapshot_sanitized.cc | 4 +++- snapshot/sanitized/process_snapshot_sanitized.h | 1 + .../sanitized/process_snapshot_sanitized_test.cc | 12 ++++++++++++ 4 files changed, 17 insertions(+), 2 deletions(-) diff --git a/DEPS b/DEPS index b7d44b11..ec7b12ea 100644 --- a/DEPS +++ b/DEPS @@ -47,7 +47,7 @@ deps = { '9719c1e1e676814c456b55f5f070eabad6709d31', 'crashpad/third_party/mini_chromium/mini_chromium': Var('chromium_git') + '/chromium/mini_chromium@' + - '10f39a97650a0fe0b305415c15434443c0690a20', + '076bcf6a916171c180f46c3487ee3e5c7bca5f20', 'crashpad/third_party/libfuzzer/src': Var('chromium_git') + '/chromium/llvm-project/compiler-rt/lib/fuzzer.git@' + 'fda403cf93ecb8792cb1d061564d89a6553ca020', diff --git a/snapshot/sanitized/module_snapshot_sanitized.cc b/snapshot/sanitized/module_snapshot_sanitized.cc index 192b4cb1..0ad2ee97 100644 --- a/snapshot/sanitized/module_snapshot_sanitized.cc +++ b/snapshot/sanitized/module_snapshot_sanitized.cc @@ -14,6 +14,8 @@ #include "snapshot/sanitized/module_snapshot_sanitized.h" +#include "base/strings/pattern.h" + namespace crashpad { namespace internal { @@ -22,7 +24,7 @@ namespace { bool KeyIsAllowed(const std::string& name, const std::vector& allowed_keys) { for (const auto& key : allowed_keys) { - if (name == key) { + if (base::MatchPattern(name, key)) { return true; } } diff --git a/snapshot/sanitized/process_snapshot_sanitized.h b/snapshot/sanitized/process_snapshot_sanitized.h index c5dfa4a0..af65fb68 100644 --- a/snapshot/sanitized/process_snapshot_sanitized.h +++ b/snapshot/sanitized/process_snapshot_sanitized.h @@ -53,6 +53,7 @@ class ProcessSnapshotSanitized final : public ProcessSnapshot { //! \param[in] allowed_annotations A list of annotations names to allow to //! be returned by AnnotationsSimpleMap() or from this object's module //! snapshots. If `nullptr`, all annotations will be returned. + // These annotation names support pattern matching, eg: "switch-*" //! \param[in] allowed_memory_ranges A list of memory ranges to allow to be //! accessible via Memory(), or `nullptr` to allow all ranges. //! \param[in] target_module_address An address in the target process' diff --git a/snapshot/sanitized/process_snapshot_sanitized_test.cc b/snapshot/sanitized/process_snapshot_sanitized_test.cc index ba1272b5..329c3c76 100644 --- a/snapshot/sanitized/process_snapshot_sanitized_test.cc +++ b/snapshot/sanitized/process_snapshot_sanitized_test.cc @@ -79,6 +79,8 @@ class ExceptionGenerator { }; constexpr char kAllowedAnnotationName[] = "name_of_allowed_anno"; +constexpr char kAllowedAnnotationNamePattern[] = "name_of_another_*"; +constexpr char kAllowedAnnotationNamePatternActual[] = "name_of_another_anno"; constexpr char kAllowedAnnotationValue[] = "some_value"; constexpr char kNonAllowedAnnotationName[] = "non_allowed_anno"; constexpr char kNonAllowedAnnotationValue[] = "private_annotation"; @@ -99,6 +101,10 @@ void ChildTestFunction() { static StringAnnotation<32> allowed_annotation(kAllowedAnnotationName); allowed_annotation.Set(kAllowedAnnotationValue); + static StringAnnotation<32> allowed_matched_annotation( + kAllowedAnnotationNamePatternActual); + allowed_matched_annotation.Set(kAllowedAnnotationValue); + static StringAnnotation<32> non_allowed_annotation(kNonAllowedAnnotationName); non_allowed_annotation.Set(kNonAllowedAnnotationValue); @@ -129,11 +135,15 @@ CRASHPAD_CHILD_TEST_MAIN(ChildToBeSanitized) { void ExpectAnnotations(ProcessSnapshot* snapshot, bool sanitized) { bool found_allowed = false; + bool found_matched_allowed = false; bool found_non_allowed = false; for (auto module : snapshot->Modules()) { for (const auto& anno : module->AnnotationObjects()) { if (anno.name == kAllowedAnnotationName) { found_allowed = true; + } + if (anno.name == kAllowedAnnotationNamePatternActual) { + found_matched_allowed = true; } else if (anno.name == kNonAllowedAnnotationName) { found_non_allowed = true; } @@ -141,6 +151,7 @@ void ExpectAnnotations(ProcessSnapshot* snapshot, bool sanitized) { } EXPECT_TRUE(found_allowed); + EXPECT_TRUE(found_matched_allowed); if (sanitized) { EXPECT_FALSE(found_non_allowed); } else { @@ -279,6 +290,7 @@ class SanitizeTest : public MultiprocessExec { auto allowed_annotations = std::make_unique>(); allowed_annotations->push_back(kAllowedAnnotationName); + allowed_annotations->push_back(kAllowedAnnotationNamePattern); auto allowed_memory_ranges = std::make_unique>>();