From c3d5696193ad9a009311dba21c13a5d759b90ce0 Mon Sep 17 00:00:00 2001
From: Aaron Leventhal <aleventhal@google.com>
Date: Tue, 14 Apr 2020 15:28:57 +0000
Subject: [Backport] Security bug 1065122

Manual backport of patch originally reviewed on
https://chromium-review.googlesource.com/c/chromium/src/+/2147813:
Add some crash debugging checks

Bug: 1065122
Change-Id: I2d73a5d5d1e9ed59f26afe10fcce421572ca7fe6
Reviewed-by: Dominic Mazzoni <dmazzoni@chromium.org>
Commit-Queue: Aaron Leventhal <aleventhal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#758849}
Reviewed-by: Michal Klocek <michal.klocek@qt.io>
---
 chromium/ui/accessibility/ax_tree_serializer.h | 33 ++++++++++++++++++++------
 1 file changed, 26 insertions(+), 7 deletions(-)

diff --git a/chromium/ui/accessibility/ax_tree_serializer.h b/chromium/ui/accessibility/ax_tree_serializer.h
index d0cd8be8dbf..21696e5433c 100644
--- a/chromium/ui/accessibility/ax_tree_serializer.h
+++ b/chromium/ui/accessibility/ax_tree_serializer.h
@@ -180,6 +180,8 @@ class AXTreeSerializer {
   // like when Reset() is called.
   void InternalReset();
 
+  ClientTreeNode* GetClientTreeNodeParent(ClientTreeNode* obj);
+
   // The tree source.
   AXTreeSource<AXSourceNode, AXNodeData, AXTreeData>* tree_;
 
@@ -269,7 +271,7 @@ AXTreeSerializer<AXSourceNode, AXNodeData, AXTreeData>::LeastCommonAncestor(
   std::vector<ClientTreeNode*> client_ancestors;
   while (client_node) {
     client_ancestors.push_back(client_node);
-    client_node = client_node->parent;
+    client_node = GetClientTreeNodeParent(client_node);
   }
 
   // Start at the root. Keep going until the source ancestor chain and
@@ -304,9 +306,12 @@ AXTreeSerializer<AXSourceNode, AXNodeData, AXTreeData>::LeastCommonAncestor(
   // that we're inside of an invalid subtree that all needs to be
   // re-serialized, so the LCA should be higher.
   ClientTreeNode* client_node = ClientTreeNodeById(tree_->GetId(node));
-  while (
-      tree_->IsValid(node) &&
-      (!client_node || (client_node->parent && client_node->parent->invalid))) {
+  while (tree_->IsValid(node)) {
+    if (client_node) {
+      ClientTreeNode* parent = GetClientTreeNodeParent(client_node);
+      if (!parent || !parent->invalid)
+        break;
+    }
     node = tree_->GetParent(node);
     if (tree_->IsValid(node))
       client_node = ClientTreeNodeById(tree_->GetId(node));
@@ -326,12 +331,13 @@ bool AXTreeSerializer<AXSourceNode, AXNodeData, AXTreeData>::
     int child_id = tree_->GetId(child);
     ClientTreeNode* client_child = ClientTreeNodeById(child_id);
     if (client_child) {
-      if (!client_child->parent) {
+      ClientTreeNode* parent = client_child->parent;
+      if (!parent) {
         // If the client child has no parent, it must have been the
         // previous root node, so there is no LCA and we can exit early.
         *out_lca = tree_->GetNull();
         return true;
-      } else if (client_child->parent->id != id) {
+      } else if (parent->id != id) {
         // If the client child's parent is not this node, update the LCA
         // and return true (reparenting was found).
         *out_lca = LeastCommonAncestor(*out_lca, client_child);
@@ -362,6 +368,19 @@ AXTreeSerializer<AXSourceNode, AXNodeData, AXTreeData>::ClientTreeNodeById(
   return nullptr;
 }
 
+template <typename AXSourceNode, typename AXNodeData, typename AXTreeData>
+ClientTreeNode*
+AXTreeSerializer<AXSourceNode, AXNodeData, AXTreeData>::GetClientTreeNodeParent(
+    ClientTreeNode* obj) {
+  ClientTreeNode* parent = obj->parent;
+#if DCHECK_IS_ON()
+  if (!parent)
+    return nullptr;
+  DCHECK(ClientTreeNodeById(parent->id)) << "Parent not in id map.";
+#endif  // DCHECK_IS_ON()
+  return parent;
+}
+
 template <typename AXSourceNode, typename AXNodeData, typename AXTreeData>
 bool AXTreeSerializer<AXSourceNode, AXNodeData, AXTreeData>::SerializeChanges(
     AXSourceNode node,
@@ -545,7 +564,7 @@ bool AXTreeSerializer<AXSourceNode, AXNodeData, AXTreeData>::
     // above. If this happens, reset and return an error.
 
     ClientTreeNode* client_child = ClientTreeNodeById(new_child_id);
-    if (client_child && client_child->parent != client_node) {
+    if (client_child && GetClientTreeNodeParent(client_child) != client_node) {
       DVLOG(1) << "Reparenting detected";
       Reset();
       return false;
-- 
cgit v1.2.1