Validate incoming window properties.

ui/aura/mus/property_converter.cc

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "ui/aura/mus/property_converter.h"
 
 #include "base/memory/ptr_util.h"
 #include "mojo/public/cpp/bindings/type_converter.h"
 #include "services/ui/public/cpp/property_type_converters.h"
 #include "services/ui/public/interfaces/window_manager.mojom.h"
 #include "ui/aura/client/aura_constants.h"
 #include "ui/base/class_property.h"
 
 namespace aura {
 
 namespace {
 
 // Get the WindowProperty's value as a byte array. Only supports aura properties
 // that point to types with a matching std::vector<uint8_t> mojo::TypeConverter.
 template <typename T>
 std::unique_ptr<std::vector<uint8_t>> GetArray(Window* window,
                                                const WindowProperty<T>* key) {
   const T value = window->GetProperty(key);
   if (!value)
     return base::MakeUnique<std::vector<uint8_t>>();
   return base::MakeUnique<std::vector<uint8_t>>(
       mojo::ConvertTo<std::vector<uint8_t>>(*value));
 }
 
+// A validator that always returns true regardless of its input.
+bool AlwaysTrue(int64_t value) {
+  return true;
+}
+
+bool ValidateResizeBehavior(int64_t value) {
+  // Resize behaviour is a 3 bitfield.

[msw, 2017/02/23 22:35:12]

nit: consistent Behavior/behaviour spelling

[Elliot Glaysher, 2017/02/23 22:57:34]

Done.

+  return value >= 0 && value <= 7;

[msw, 2017/02/23 22:35:12]

optional nit: use value >= 0 && value <= ui::mojom::kResizeBehaviorCanMaximize |
ui::mojom::kResizeBehaviorCanMinimize | ui::mojom::kResizeBehaviorCanResize;

[Elliot Glaysher, 2017/02/23 22:57:34]

Done.

+}
+
+bool ValidateShowState(int64_t value) {
+  return value == int64_t(ui::mojom::ShowState::DEFAULT) ||

[msw, 2017/02/23 22:35:12]

It's a bummer that validating each enum requires listing all values.

[Elliot Glaysher, 2017/02/23 22:57:34]

Acknowledged.

+         value == int64_t(ui::mojom::ShowState::NORMAL) ||
+         value == int64_t(ui::mojom::ShowState::MINIMIZED) ||
+         value == int64_t(ui::mojom::ShowState::MAXIMIZED) ||
+         value == int64_t(ui::mojom::ShowState::INACTIVE) ||
+         value == int64_t(ui::mojom::ShowState::FULLSCREEN) ||
+         value == int64_t(ui::mojom::ShowState::DOCKED);
+}
+
 }  // namespace
 
+PropertyConverter::PrimitiveProperty::PrimitiveProperty() {}
+
+PropertyConverter::PrimitiveProperty::PrimitiveProperty(
+    const PrimitiveProperty& property) = default;
+
+PropertyConverter::PrimitiveProperty::~PrimitiveProperty() {}
+
+// static
+base::RepeatingCallback<bool(int64_t)>
+PropertyConverter::CreateAcceptAnyValueCallback() {
+  return base::Bind(&AlwaysTrue);
+}
+
 PropertyConverter::PropertyConverter() {
   // Add known aura properties with associated mus properties.
   RegisterProperty(client::kAlwaysOnTopKey,
-                   ui::mojom::WindowManager::kAlwaysOnTop_Property);
+                   ui::mojom::WindowManager::kAlwaysOnTop_Property,
+                   CreateAcceptAnyValueCallback());

[msw, 2017/02/23 22:35:11]

Should we have a special callback for validating bool values?

[Elliot Glaysher, 2017/02/23 22:57:35]

Unsure; are there cases where a non-0, non-1 value is used as true?

[msw, 2017/02/23 23:22:35]

Yeah, perhaps there's no value in doing so... I was just pondering.

   RegisterProperty(client::kAppIconKey,
                    ui::mojom::WindowManager::kAppIcon_Property);
   RegisterProperty(client::kAppIdKey,
                    ui::mojom::WindowManager::kAppID_Property);
   RegisterProperty(client::kImmersiveFullscreenKey,
-                   ui::mojom::WindowManager::kImmersiveFullscreen_Property);
+                   ui::mojom::WindowManager::kImmersiveFullscreen_Property,
+                   CreateAcceptAnyValueCallback());
   RegisterProperty(client::kNameKey, ui::mojom::WindowManager::kName_Property);
   RegisterProperty(client::kPreferredSize,
                    ui::mojom::WindowManager::kPreferredSize_Property);
   RegisterProperty(client::kResizeBehaviorKey,
-                   ui::mojom::WindowManager::kResizeBehavior_Property);
+                   ui::mojom::WindowManager::kResizeBehavior_Property,
+                   base::Bind(&ValidateResizeBehavior));
   RegisterProperty(client::kRestoreBoundsKey,
                    ui::mojom::WindowManager::kRestoreBounds_Property);
   RegisterProperty(client::kShowStateKey,
-                   ui::mojom::WindowManager::kShowState_Property);
+                   ui::mojom::WindowManager::kShowState_Property,
+                   base::Bind(&ValidateShowState));
   RegisterProperty(client::kWindowIconKey,
                    ui::mojom::WindowManager::kWindowIcon_Property);
   RegisterProperty(client::kTitleKey,
                    ui::mojom::WindowManager::kWindowTitle_Property);
 }
 
 PropertyConverter::~PropertyConverter() {}
 
 bool PropertyConverter::IsTransportNameRegistered(
     const std::string& name) const {
@@ skipping 89 matching lines @@
     const std::string& transport_name,
     const std::vector<uint8_t>* data) {
   for (const auto& primitive_property : primitive_properties_) {
     if (primitive_property.second.transport_name == transport_name) {
       // aura::Window only supports property types that fit in PrimitiveType.
       if (data->size() != 8u) {
         DVLOG(2) << "Property size mismatch (PrimitiveType): "
                  << transport_name;
         return;
       }
+      const int64_t v = *reinterpret_cast<const int64_t*>(&data->front());
+      if (!primitive_property.second.validator.Run(v))
+        return;

[msw, 2017/02/23 22:35:12]

We should probably log when invalid property values are encountered.

[Elliot Glaysher, 2017/02/23 22:57:35]

Done.

       const PrimitiveType value = mojo::ConvertTo<PrimitiveType>(*data);
       // TODO(msw): Should aura::Window just store all properties by name?
       window->SetPropertyInternal(
           primitive_property.first, primitive_property.second.property_name,
           nullptr, value, primitive_property.second.default_value);
       return;
     }
   }
 
   for (const auto& image_property : image_properties_) {
@@ skipping 56 matching lines @@
     const std::string& transport_name,
     const std::vector<uint8_t>& transport_data,
     PrimitiveType* value) {
   // aura::Window only supports property types that fit in PrimitiveType.
   if (transport_data.size() != 8u) {
     DVLOG(2) << "Property size mismatch (PrimitiveType): " << transport_name;
     return false;
   }
   for (const auto& primitive_property : primitive_properties_) {
     if (primitive_property.second.transport_name == transport_name) {
+      const int64_t v =
+          *reinterpret_cast<const int64_t*>(&transport_data.front());
+      if (!primitive_property.second.validator.Run(v))
+        return false;

[msw, 2017/02/23 22:35:12]

Ditto

[Elliot Glaysher, 2017/02/23 22:57:35]

Done.

       *value = mojo::ConvertTo<PrimitiveType>(transport_data);
       return true;
     }
   }
   return false;
 }
 
 void PropertyConverter::RegisterProperty(
     const WindowProperty<gfx::ImageSkia*>* property,
     const char* transport_name) {
@@ skipping 23 matching lines @@
 }
 
 void PropertyConverter::RegisterProperty(
     const WindowProperty<base::string16*>* property,
     const char* transport_name) {
   string16_properties_[property] = transport_name;
   transport_names_.insert(transport_name);
 }
 
 }  // namespace aura