Another attempt at fixing the flash security issue surrounding

window.location.

This change disallows shadowing of window and location both when using
var declarations and when defining accessors.  Using 'var location =
stuff' will cause a navigation.  This is consistent with what Firefox
does.

Also, introduce an explicit valueOf function on location objects so
that implicit string conversions cannot be intercepted to fake the
current location reported by window.location.
Review URL: http://codereview.chromium.org/9636

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@4979 0039d316-1c4b-4281-b951-d872f2087c98

6 files changed, 194 insertions, 109 deletions

diff --git a/webkit/port/bindings/scripts/CodeGeneratorV8.pm b/webkit/port/bindings/scripts/CodeGeneratorV8.pm
index 1526b88..5d0d110 100644
--- a/webkit/port/bindings/scripts/CodeGeneratorV8.pm
+++ b/webkit/port/bindings/scripts/CodeGeneratorV8.pm
@@ -279,6 +279,16 @@ sub GenerateHeader
  public:
   static bool HasInstance(v8::Handle<v8::Value> value);
   static v8::Persistent<v8::FunctionTemplate> GetRawTemplate();
+END
+
+    if ($implClassName eq "DOMWindow") {
+      push(@headerContent, <<END);
+  static v8::Persistent<v8::ObjectTemplate> GetShadowObjectTemplate();
+END
+    }
+
+    push(@headerContent, <<END);
+
  private:
   static v8::Persistent<v8::FunctionTemplate> GetTemplate();
   
@@ -471,7 +481,7 @@ END
 END
     }
 
-  } elsif ($attrExt->{"v8OnProto"}) {
+  } elsif ($attrExt->{"v8OnProto"} || $attrExt->{"v8DisallowShadowing"}) {
     # perform lookup first
     push(@implContentDecls, <<END);
     v8::Handle<v8::Object> holder = V8Proxy::LookupDOMWrapper(V8ClassIndex::$classIndex, info.This());
@@ -828,6 +838,115 @@ END
 }
 
 
+sub GenerateBatchedAttributeData
+{
+  my $interfaceName = shift;
+  my $attributes = shift;
+
+  foreach my $attribute (@$attributes) {
+    my $attrName = $attribute->signature->name;
+    my $attrExt = $attribute->signature->extendedAttributes;
+
+    my $accessControl = "v8::DEFAULT";
+    if ($attrExt->{"DoNotCheckDomainSecurityOnGet"}) {
+      $accessControl = "v8::ALL_CAN_READ";
+    } elsif ($attrExt->{"DoNotCheckDomainSecurityOnSet"}) {
+      $accessControl = "v8::ALL_CAN_WRITE";
+    } elsif ($attrExt->{"DoNotCheckDomainSecurity"}) {
+      $accessControl = "v8::ALL_CAN_READ";
+       if (!($attribute->type =~ /^readonly/)) {
+         $accessControl .= "|v8::ALL_CAN_WRITE";
+       }
+    }
+    if ($attrExt->{"v8DisallowShadowing"}) {
+      $interfaceName eq "DOMWindow" || die "v8DisallowShadowing can only be used on the DOMWindow interface.";
+      $accessControl .= "|v8::PROHIBITS_OVERWRITING";
+    }
+    $accessControl = "static_cast<v8::AccessControl>(" . $accessControl . ")";
+
+
+    my $customAccessor = $attrExt->{"Custom"} || $attrExt->{"CustomSetter"} || $attrExt->{"CustomGetter"} || "";
+    if ($customAccessor eq 1) {
+      # use the naming convension, interface + (capitalize) attr name
+      $customAccessor = $interfaceName . WK_ucfirst($attrName);
+    }
+  
+    my $getter;
+    my $setter;
+    my $propAttr = "v8::None";
+
+    # Check enumerability.
+    if ($attribute->signature->extendedAttributes->{"DontEnum"}) {
+      $propAttr .= "|v8::DontEnum";
+    }
+
+    my $on_proto = "0 /* on instance */";
+    my $data = "V8ClassIndex::INVALID_CLASS_INDEX /* no data */";
+
+    # Constructor
+    if ($attribute->signature->type =~ /Constructor$/) {
+      my $constructorType = $codeGenerator->StripModule($attribute->signature->type);
+      $constructorType =~ s/Constructor$//;
+      my $constructorIndex = uc($constructorType);
+      $data = "V8ClassIndex::${constructorIndex}";
+      $getter = "${interfaceName}Internal::${interfaceName}ConstructorGetter";
+      $setter = "0";
+      $propAttr = "v8::ReadOnly";
+    
+    # Custom Getter and Setter
+    } elsif ($attrExt->{"Custom"}) {
+      $getter = "V8Custom::v8${customAccessor}AccessorGetter";
+      $setter = "V8Custom::v8${customAccessor}AccessorSetter";
+      
+    # Custom Setter
+    } elsif ($attrExt->{"CustomSetter"}) {
+      $getter = "${interfaceName}Internal::${attrName}AttrGetter";
+      $setter = "V8Custom::v8${customAccessor}AccessorSetter";
+
+    # Custom Getter
+    } elsif ($attrExt->{"CustomGetter"}) {
+      $getter = "V8Custom::v8${customAccessor}AccessorGetter";
+      $setter = "${interfaceName}Internal::${attrName}AttrSetter";
+              
+    # Replaceable
+    } elsif ($attrExt->{"Replaceable"}) {
+      # Replaceable accessor is put on instance template with ReadOnly attribute.
+      $getter = "${interfaceName}Internal::${attrName}AttrGetter";
+      $setter = "0";
+      $propAttr .= "|v8::ReadOnly";
+      
+    # Normal
+    } else {
+      $getter = "${interfaceName}Internal::${attrName}AttrGetter";
+      $setter = "${interfaceName}Internal::${attrName}AttrSetter";
+    }
+
+    # Read only attributes
+    if ($attribute->type =~ /^readonly/) {
+      $setter = "0";
+    }
+
+    # An accessor can be installed on the proto
+    if ($attrExt->{"v8OnProto"}) {
+      $on_proto = "1 /* on proto */";
+    }
+
+    my $commentInfo = "Attribute '$attrName' (Type: '" . $attribute->type .
+                      "' ExtAttr: '" . join(' ', keys(%{$attrExt})) . "')";
+    push(@implContent, <<END);
+  // $commentInfo
+  { "$attrName",
+    $getter,
+    $setter,
+    $data,
+    $accessControl,
+    static_cast<v8::PropertyAttribute>($propAttr),
+    $on_proto },
+END
+    }
+}
+
+
 sub GenerateImplementation
 {
     my $object = shift;
@@ -947,112 +1066,33 @@ sub GenerateImplementation
     }
 
     # Attributes
-    my $has_attributes = 0;
-    if (@{$dataNode->attributes}) {
-      $has_attributes = 1;
-      push(@implContent, "static const BatchedAttribute attrs[] = {\n");
-    }
-    foreach my $attribute (@{$dataNode->attributes}) {
-      my $attrName = $attribute->signature->name;
-      my $attrExt = $attribute->signature->extendedAttributes;
-
-      my $accessControl = "v8::DEFAULT";
-      if ($attrExt->{"DoNotCheckDomainSecurityOnGet"}) {
-        $accessControl = "v8::ALL_CAN_READ";
-      } elsif ($attrExt->{"DoNotCheckDomainSecurityOnSet"}) {
-        $accessControl = "v8::ALL_CAN_WRITE";
-      } elsif ($attrExt->{"DoNotCheckDomainSecurity"}) {
-        $accessControl = "v8::ALL_CAN_READ";
-         if (!($attribute->type =~ /^readonly/)) {
-           $accessControl .= "|v8::ALL_CAN_WRITE";
-         }
-      }
-      if ($attrExt->{"v8ProhibitsOverwriting"}) {
-        $accessControl .= "|v8::PROHIBITS_OVERWRITING";
-      }
-      $accessControl = "static_cast<v8::AccessControl>(" . $accessControl . ")";
-
-
-      my $customAccessor = $attrExt->{"Custom"} || $attrExt->{"CustomSetter"} || $attrExt->{"CustomGetter"} || "";
-      if ($customAccessor eq 1) {
-        # use the naming convension, interface + (capitalize) attr name
-        $customAccessor = $interfaceName . WK_ucfirst($attrName);
-      }
-    
-      my $getter;
-      my $setter;
-      my $propAttr = "v8::None";
-
-      # Check enumerability.
-      if ($attribute->signature->extendedAttributes->{"DontEnum"}) {
-        $propAttr .= "|v8::DontEnum";
-      }
-
-      my $on_proto = "0 /* on instance */";
-      my $data = "V8ClassIndex::INVALID_CLASS_INDEX /* no data */";
-
-      # Constructor
-      if ($attribute->signature->type =~ /Constructor$/) {
-        my $constructorType = $codeGenerator->StripModule($attribute->signature->type);
-        $constructorType =~ s/Constructor$//;
-        my $constructorIndex = uc($constructorType);
-        $data = "V8ClassIndex::${constructorIndex}";
-        $getter = "${interfaceName}Internal::${implClassName}ConstructorGetter";
-        $setter = "0";
-        $propAttr = "v8::ReadOnly";
-      
-      # Custom Getter and Setter
-      } elsif ($attrExt->{"Custom"}) {
-        $getter = "V8Custom::v8${customAccessor}AccessorGetter";
-        $setter = "V8Custom::v8${customAccessor}AccessorSetter";
-        
-      # Custom Setter
-      } elsif ($attrExt->{"CustomSetter"}) {
-        $getter = "${interfaceName}Internal::${attrName}AttrGetter";
-        $setter = "V8Custom::v8${customAccessor}AccessorSetter";
-
-      # Custom Getter
-      } elsif ($attrExt->{"CustomGetter"}) {
-        $getter = "V8Custom::v8${customAccessor}AccessorGetter";
-        $setter = "${interfaceName}Internal::${attrName}AttrSetter";
-                
-      # Replaceable
-      } elsif ($attrExt->{"Replaceable"}) {
-        # Replaceable accessor is put on instance template with ReadOnly attribute.
-        $getter = "${interfaceName}Internal::${attrName}AttrGetter";
-        $setter = "0";
-        $propAttr .= "|v8::ReadOnly";
-        
-      # Normal
-      } else {
-        $getter = "${interfaceName}Internal::${attrName}AttrGetter";
-        $setter = "${interfaceName}Internal::${attrName}AttrSetter";
-      }
-
-      # Read only attributes
-      if ($attribute->type =~ /^readonly/) {
-        $setter = "0";
-      }
-
-      # An accessor can be installed on the proto
-      if ($attrExt->{"v8OnProto"}) {
-        $on_proto = "1 /* on proto */";
+    my $attributes = $dataNode->attributes;
+
+    # For the DOMWindow interface we partition the attributes into the
+    # ones that disallows shadowing and the rest.
+    my @disallows_shadowing;
+    my @normal;
+    if ($interfaceName eq "DOMWindow") {
+      foreach my $attribute (@$attributes) {
+        if ($attribute->signature->extendedAttributes->{"v8DisallowShadowing"}) {
+          push(@disallows_shadowing, $attribute);
+        } else {
+          push(@normal, $attribute);
+        }
       }
+      # Put the attributes that disallow shadowing on the shadow object.
+      $attributes = \@normal;
+      push(@implContent, "static const BatchedAttribute shadow_attrs[] = {\n");
+      GenerateBatchedAttributeData($interfaceName, \@disallows_shadowing);
+      push(@implContent, "};\n");
 
-      my $commentInfo = "Attribute '$attrName' (Type: '" . $attribute->type .
-                        "' ExtAttr: '" . join(' ', keys(%{$attrExt})) . "')";
-      push(@implContent, <<END);
-  // $commentInfo
-  { "$attrName",
-    $getter,
-    $setter,
-    $data,
-    $accessControl,
-    static_cast<v8::PropertyAttribute>($propAttr),
-    $on_proto },
-END
     }
-    if ($has_attributes) {
+
+    my $has_attributes = 0;
+    if (@$attributes) {
+      $has_attributes = 1;
+      push(@implContent, "static const BatchedAttribute attrs[] = {\n");
+      GenerateBatchedAttributeData($interfaceName, $attributes);
       push(@implContent, "};\n");
     }
 
@@ -1083,8 +1123,22 @@ END
       $access_check = "instance->SetAccessCheckCallbacks(V8Custom::v8${interfaceName}NamedSecurityCheck, V8Custom::v8${interfaceName}IndexedSecurityCheck, v8::Integer::New(V8ClassIndex::ToInt(V8ClassIndex::${classIndex})));";
     }
 
+    # For the DOMWindow interface, generate the shadow object template
+    # configuration method.
+    if ($implClassName eq "DOMWindow") {
+      push(@implContent, <<END);
+static v8::Persistent<v8::ObjectTemplate> ConfigureShadowObjectTemplate(v8::Persistent<v8::ObjectTemplate> templ) {
+  BatchConfigureAttributes(templ,
+                           v8::Handle<v8::ObjectTemplate>(),
+                           shadow_attrs,
+                           sizeof(shadow_attrs)/sizeof(*shadow_attrs));
+  return templ;
+}
+END
+    }
+
     # Generate the template configuration method
-    push(@implContent,  <<END
+    push(@implContent,  <<END);
 static v8::Persistent<v8::FunctionTemplate> Configure${className}Template(v8::Persistent<v8::FunctionTemplate> desc) {
   v8::Local<v8::ObjectTemplate> instance = desc->InstanceTemplate();
   instance->SetInternalFieldCount(2);
@@ -1092,7 +1146,7 @@ static v8::Persistent<v8::FunctionTemplate> Configure${className}Template(v8::Pe
   v8::Local<v8::ObjectTemplate> proto = desc->PrototypeTemplate();
   $access_check
 END
-);
+
 
     # Set up our attributes if we have them
     if ($has_attributes) {
@@ -1223,7 +1277,22 @@ bool ${className}::HasInstance(v8::Handle<v8::Value> value) {
   return GetRawTemplate()->HasInstance(value);
 }
 
+END
 
+    if ($implClassName eq "DOMWindow") {
+      push(@implContent, <<END);
+v8::Persistent<v8::ObjectTemplate> V8DOMWindow::GetShadowObjectTemplate() {
+  static v8::Persistent<v8::ObjectTemplate> V8DOMWindowShadowObject_cache_;
+  if (V8DOMWindowShadowObject_cache_.IsEmpty()) {
+    V8DOMWindowShadowObject_cache_ = v8::Persistent<v8::ObjectTemplate>::New(v8::ObjectTemplate::New());
+    ConfigureShadowObjectTemplate(V8DOMWindowShadowObject_cache_);
+  }
+  return V8DOMWindowShadowObject_cache_;
+}
+END
+    }
+
+    push(@implContent, <<END);
 } // namespace WebCore 
 END
 
diff --git a/webkit/port/bindings/v8/v8_custom.cpp b/webkit/port/bindings/v8/v8_custom.cpp
index 9bb904d..23a226d 100644
--- a/webkit/port/bindings/v8/v8_custom.cpp
+++ b/webkit/port/bindings/v8/v8_custom.cpp
@@ -2626,6 +2626,16 @@ CALLBACK_FUNC_DECL(ElementSetAttributeNodeNS) {
   return V8Proxy::NodeToV8Object(result.get());
 }
 
+// Location --------------------------------------------------------------------
+
+CALLBACK_FUNC_DECL(LocationValueOf) {
+  // Just return the this object the way the normal valueOf function
+  // on the Object prototype would.  The valueOf function is only
+  // added to make sure that it cannot be overwritten on location
+  // objects, since that would provide a hook to change the string
+  // conversion behavior of location objects.
+  return args.This();
+}
 
 // Attr ------------------------------------------------------------------------
 
diff --git a/webkit/port/bindings/v8/v8_custom.h b/webkit/port/bindings/v8/v8_custom.h
index ef2fb4d..6ce3239 100644
--- a/webkit/port/bindings/v8/v8_custom.h
+++ b/webkit/port/bindings/v8/v8_custom.h
@@ -286,6 +286,9 @@ DECLARE_CALLBACK(ElementSetAttributeNode)
 DECLARE_CALLBACK(ElementSetAttributeNS)
 DECLARE_CALLBACK(ElementSetAttributeNodeNS)
 
+// Implementation of custom Location methods.
+DECLARE_CALLBACK(LocationValueOf)
+
 // Implementation of EventTarget::addEventListener
 // and EventTarget::removeEventListener
 DECLARE_CALLBACK(EventTargetNodeAddEventListener)
diff --git a/webkit/port/bindings/v8/v8_proxy.cpp b/webkit/port/bindings/v8/v8_proxy.cpp
index 8693714..eb6250a 100644
--- a/webkit/port/bindings/v8/v8_proxy.cpp
+++ b/webkit/port/bindings/v8/v8_proxy.cpp
@@ -39,6 +39,7 @@
 #include "v8_custom.h"
 #include "v8_collection.h"
 #include "v8_nodefilter.h"
+#include "V8DOMWindow.h"
 
 #include "RefCounted.h"  // for Peerable
 
@@ -1812,7 +1813,8 @@ void V8Proxy::initContextIfNeeded()
 
   // Create a new environment using an empty template for the shadow
   // object.  Reuse the global object if one has been created earlier.
-  v8::Local<v8::ObjectTemplate> global_template = v8::ObjectTemplate::New();
+  v8::Persistent<v8::ObjectTemplate> global_template =
+    V8DOMWindow::GetShadowObjectTemplate();
   if (global_template.IsEmpty())
     return;
 
diff --git a/webkit/port/page/DOMWindow.idl b/webkit/port/page/DOMWindow.idl
index 0467082..a667251 100644
--- a/webkit/port/page/DOMWindow.idl
+++ b/webkit/port/page/DOMWindow.idl
@@ -52,7 +52,7 @@ module window {
         attribute [Replaceable] BarInfo toolbar;
         attribute [Replaceable] Navigator navigator;
         attribute [Replaceable, v8referenceattr=navigator] Navigator clientInformation;
-        attribute [DoNotCheckDomainSecurity, CustomSetter, v8ProhibitsOverwriting] Location location;
+        attribute [DoNotCheckDomainSecurity, CustomSetter, v8DisallowShadowing] Location location;
 
         DOMSelection getSelection();
 
@@ -116,7 +116,7 @@ module window {
 
         // Self referential attributes
         attribute [Replaceable, DoNotCheckDomainSecurityOnGet] DOMWindow self;
-        readonly attribute [DoNotCheckDomainSecurity] DOMWindow window;
+        readonly attribute [DoNotCheckDomainSecurity, v8DisallowShadowing] DOMWindow window;
         attribute [Replaceable, DoNotCheckDomainSecurityOnGet] DOMWindow frames;
 
 #if defined(V8_BINDING)
diff --git a/webkit/port/page/Location.idl b/webkit/port/page/Location.idl
index 092b81d..73acabe 100644
--- a/webkit/port/page/Location.idl
+++ b/webkit/port/page/Location.idl
@@ -20,5 +20,6 @@ module core {
     [DoNotCheckDomainSecurity, v8OnInstance] void replace(in DOMString url);
     [DoNotCheckDomainSecurity, v8OnInstance] void assign(in DOMString url);
     [ReadOnly, DontEnum, v8OnInstance] DOMString toString();
+    [Custom, ReadOnly, DontEnum, v8OnInstance] DOMObject valueOf();
   };
 }