[Extensions] Update last_error.js, send_request.js

extensions/renderer/resources/last_error.js

 // Copyright 2014 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.
 
 var GetAvailability = requireNative('v8_context').GetAvailability;
 var GetGlobal = requireNative('sendRequest').GetGlobal;
 
 // Utility for setting chrome.*.lastError.
 //
 // A utility here is useful for two reasons:
@@ skipping 50 matching lines @@
  * Check whether there is an error set on |targetChrome| without setting
  * |accessedLastError|.
  * @param {Object} targetChrome the Chrome object to check.
  * @return boolean Whether lastError has been set.
  */
 function hasError(targetChrome) {
   if (!targetChrome)
     throw new Error('No target chrome to check');
 
   assertRuntimeIsAvailable();
-  if ('lastError' in targetChrome.runtime)
-    return true;
-
-  return false;
+  return $Object.hasOwnProperty(targetChrome.runtime, 'lastError');

[robwu, 2016/04/23 09:54:38]

Note that |targetChrome| is fully under control of a third party, so this does
not offer any protection against tampering (anything can be intercepted through
a Proxy, including $Object.defineProperty).

If you find that worrisome, then you could save the generated runtime bindings
in runtime_custom_bindings.js to privates of the global object, and retrieve it
here.

But I wouldn't bother, since maybe people are relying on being able to intercept
chrome.runtime.lastError for logging or something, and there is no sensitive
information being exposed here.

[Devlin, 2016/04/25 21:50:35]

Yeah, this change was just because IIRC, 'in' actually relies on built-in object
methods, which means that it fails if someone clobbers the Object prototype (so
just trying to be a good citizen and use safe builtins here).

It'd actually be nice to get rid of targetChrome for other reasons, but I don't
think we can - we let the caller pass in an arbitrary chrome because we allow
cross-context (though not cross origin) chrome calls, and the error should be
set on the calling chrome.  See handleResponse() in send_request.js, and
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/test/data/e...
for an example of a cross-context call.

I'd love to be able to disallow that, but, well, it's probably too late (and at
least orthogonal to the rest of these issues).

[robwu, 2016/04/25 22:25:53]

hasOwnProperty is better than "in" because it only looks on the object itself,
but if the object is under control of a third party, then hasOwnProperty can
also be trapped, FYI:

chrome = {};
chrome.runtime = new Proxy({
  __proto__: null, // Obviously no "lastError" here.
}, {
  getOwnPropertyDescriptor() {
    return {configurable:true};
  },
});
$Object.hasOwnProperty(chrome.runtime, 'lastError'); // true

[Devlin, 2016/04/25 22:35:00]

Right, again this isn't to defend against anything other than clobbering,
because we don't have much control over targetChrome (so anything we do here is
pretty much irrelevant).

 };
 
 /**
  * Clears the last error on |targetChrome|.
  */
 function clear(targetChrome) {
   if (!targetChrome)
     throw new Error('No target chrome to clear error');
 
   if (GetAvailability('extension.lastError').is_available)
@@ skipping 49 matching lines @@
   console.error("Unchecked runtime.lastError while running " +
       (name || "unknown") + ": " + message + (stack ? "\n" + stack : ""));
 }
 
 exports.$set('clear', clear);
 exports.$set('hasAccessed', hasAccessed);
 exports.$set('hasError', hasError);
 exports.$set('set', set);
 exports.$set('run', run);
 exports.$set('reportIfUnchecked', reportIfUnchecked);