Validate untrusted VR mojo inputs into browser process

third_party/WebKit/Source/modules/vr/VRDisplay.cpp

 // 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 "modules/vr/VRDisplay.h"
 
 #include "core/css/StylePropertySet.h"
 #include "core/dom/DOMException.h"
 #include "core/dom/FrameRequestCallback.h"
 #include "core/dom/ScriptedAnimationController.h"
@@ skipping 354 matching lines @@
       (layer_.rightBounds().size() != 0 && layer_.rightBounds().size() != 4)) {
     ForceExitPresent();
     DOMException* exception = DOMException::Create(
         kInvalidStateError,
         "Layer bounds must either be an empty array or have 4 values");
     resolver->Reject(exception);
     ReportPresentationResult(PresentationResult::kInvalidLayerBounds);
     return promise;
   }
 
+  for (float value : layer_.leftBounds()) {

[mthiesse, 2017/07/11 14:52:24]

Should move these checks into UpdateLayerBounds(), and return an error or
something so that the caller can reject the promise, as UpdateLayerBounds is
called from multiple places.

[billorr, 2017/07/13 00:31:41]

It is called from multiple places, but this is the only place we are consuming
data from callers - I'd prefer to keep the validation close to where data comes
from javascript.  This is where other validation of layer_ is performed.

Also, we don't call UpdateLayerBounds for a new presentation until BeginPresent,
which is after GVR has started up.

+    if (std::isnan(value)) {
+      ForceExitPresent();
+      DOMException* exception = DOMException::Create(
+          kInvalidStateError, "Layer bounds must not contain NAN values");
+      resolver->Reject(exception);
+      ReportPresentationResult(PresentationResult::kInvalidLayerBounds);
+      return promise;
+    }
+  }
+
+  for (float value : layer_.rightBounds()) {
+    if (std::isnan(value)) {
+      ForceExitPresent();
+      DOMException* exception = DOMException::Create(
+          kInvalidStateError, "Layer bounds must not contain NAN values");
+      resolver->Reject(exception);
+      ReportPresentationResult(PresentationResult::kInvalidLayerBounds);
+      return promise;
+    }
+  }
+

[mthiesse, 2017/07/11 14:52:24]

Also check the width/height to be >= 0 to avoid unnecessary crashes.

[billorr, 2017/07/13 00:31:41]

I removed the size check on the other side.  We already check for size.width !=
0 and size.height != 0.  The magic of mojo turns the WebSize into a gfx::size,
which will make width and height = 0 if negative.

   if (!pending_present_resolvers_.IsEmpty()) {
     // If we are waiting on the results of a previous requestPresent call don't
     // fire a new request, just cache the resolver and resolve it when the
     // original request returns.
     pending_present_resolvers_.push_back(resolver);
   } else if (first_present) {
     bool secure_context =
         ExecutionContext::From(script_state)->IsSecureContext();
     if (!display_) {
       ForceExitPresent();
@@ skipping 305 matching lines @@
 
   pending_previous_frame_render_ = true;
   pending_submit_frame_ = true;
 
   TRACE_EVENT_BEGIN0("gpu", "VRDisplay::SubmitFrame");
   vr_presentation_provider_->SubmitFrame(
       vr_frame_id_, gpu::MailboxHolder(mailbox, sync_token, GL_TEXTURE_2D));
   TRACE_EVENT_END0("gpu", "VRDisplay::SubmitFrame");
 
   did_submit_this_frame_ = true;
+  // Reset our frame id, since anything we'd want to do (resizing/etc) can
+  // no-longer happen to this frame.
+  vr_frame_id_ = -1;
   // If we were deferring a rAF-triggered vsync request, do this now.
   RequestVSync();
 
   // If preserveDrawingBuffer is false, must clear now. Normally this
   // happens as part of compositing, but that's not active while
   // presenting, so run the responsible code directly.
   rendering_context_->MarkCompositedAndClearBackbufferIfNeeded();
 }
 
 void VRDisplay::OnSubmitFrameTransferred() {
@@ skipping 278 matching lines @@
   visitor->Trace(stage_parameters_);
   visitor->Trace(eye_parameters_left_);
   visitor->Trace(eye_parameters_right_);
   visitor->Trace(layer_);
   visitor->Trace(rendering_context_);
   visitor->Trace(scripted_animation_controller_);
   visitor->Trace(pending_present_resolvers_);
 }
 
 }  // namespace blink