Revision: 5613
Author: [email protected]
Date: Mon Oct 14 20:51:59 2013 UTC
Log: Remove ES5/3 artifacts from taming membrane
https://codereview.appspot.com/14605043
[email protected]
http://code.google.com/p/google-caja/source/detail?r=5613
Modified:
/trunk/src/com/google/caja/plugin/ses-frame-group.js
/trunk/src/com/google/caja/plugin/taming-membrane.js
/trunk/src/com/google/caja/plugin/taming-schema.js
=======================================
--- /trunk/src/com/google/caja/plugin/ses-frame-group.js Tue Oct 8
22:01:03 2013 UTC
+++ /trunk/src/com/google/caja/plugin/ses-frame-group.js Mon Oct 14
20:51:59 2013 UTC
@@ -35,27 +35,30 @@
tamingWin.ses.mitigateSrcGotchas = additionalParams.mitigateSrcGotchas;
- // TODO(kpreid): With the death of ES5/3, can we now reliably just use
- // undefined instead of USELESS always?
- var USELESS = Object.freeze({ USELESS: 'USELESS' });
- var BASE_OBJECT_CONSTRUCTOR = Object.freeze({});
+ // CAUTION: It is ESSENTIAL that we pass USELESS, not (void 0), when
+ // calling down to a feral function. That function may not be declared
+ // in "strict" mode, and so would receive [window] as its "this" arg if
+ // we called it with (void 0). This could lead to a vulnerability if the
+ // function called happened to modify its "this" arg in some way that an
+ // attacker could redirect into an attack on the global [window].
+ var USELESS = Object.freeze({
+ USELESS: 'USELESS',
+ toString: function() { return '[Caja USELESS object]'; }
+ });
- // TODO(kpreid): Review this for pieces which are no longer necessary in
the
- // post-ES5/3 world.
var tamingHelper = Object.freeze({
- applyFunction: applyFunction,
- getProperty: getProperty,
- setProperty: setProperty,
- getOwnPropertyNames: getOwnPropertyNames,
- directConstructor: directConstructor,
- getObjectCtorFor: getObjectCtorFor,
isDefinedInCajaFrame: cajaFrameTracker.isDefinedInCajaFrame,
USELESS: USELESS,
- BASE_OBJECT_CONSTRUCTOR: BASE_OBJECT_CONSTRUCTOR,
- getValueOf: function(o) { return o.valueOf(); },
- weakMapPermitHostObjects: ses.weakMapPermitHostObjects
+ weakMapPermitHostObjects: ses.weakMapPermitHostObjects,
+ allFrames: allFrames
});
+ function allFrames() {
+ var a = Array.prototype.slice.call(feralWin.frames);
+ a.push(feralWin);
+ return a;
+ }
+
var frameGroupTamingSchema = TamingSchema(tamingHelper);
var frameGroupTamingMembrane =
TamingMembrane(tamingHelper, frameGroupTamingSchema.control);
@@ -119,6 +122,7 @@
}
function makeDefensibleFunction(f) {
+ // See notes on USELESS above
return Object.freeze(function() {
return f.apply(USELESS, Array.prototype.slice.call(arguments, 0));
});
@@ -135,77 +139,6 @@
function setProperty(o, p, v) {
return o[p] = v;
}
-
- function directConstructor(obj) {
- if (obj === null) { return void 0; }
- if (obj === void 0) { return void 0; }
- if ((typeof obj) !== 'object') {
- // Regarding functions, since functions return undefined,
- // directConstructor() doesn't provide access to the
- // forbidden Function constructor.
- // Otherwise, we don't support finding the direct constructor
- // of a primitive.
- return void 0;
- }
- var directProto = Object.getPrototypeOf(obj);
- if (!directProto) { return void 0; }
- var directCtor = directProto.constructor;
- if (!directCtor) { return void 0; }
- if (directCtor === feralWin.Object) {
- if
(!Object.prototype.hasOwnProperty.call(directProto, 'constructor')) {
- // detect prototypes which just didn't bother to set .constructor
and
- // inherited it from Object (Safari's DOMException is the
motivating
- // case).
- // Ditto for loop below.
- return void 0;
- } else {
- return BASE_OBJECT_CONSTRUCTOR;
- }
- }
- Array.prototype.slice.call(feralWin.frames).forEach(function(w) {
- var O;
- try {
- O = w.Object;
- } catch (e) {
- // met a different-origin frame, probably
- return;
- }
- if (directCtor === O) {
- if
(!Object.prototype.hasOwnProperty.call(directProto, 'constructor')) {
- directCtor = void 0;
- } else {
- directCtor = BASE_OBJECT_CONSTRUCTOR;
- }
- }
- });
- return directCtor;
- }
-
- function getObjectCtorFor(o) {
- if (o === undefined || o === null) {
- return void 0;
- }
- var ot = typeof o;
- if (ot !== 'object' && ot !== 'function') {
- throw new TypeError('Cannot obtain ctor for non-object');
- }
- var proto = undefined;
- while (o) {
- proto = o;
- o = Object.getPrototypeOf(o);
- }
- return proto.constructor;
- }
-
- function getOwnPropertyNames(o) {
- var r = [];
- Object.getOwnPropertyNames(o).forEach(function(p) {
- if (Object.getOwnPropertyDescriptor(o, p).enumerable) {
- r.push(p);
- }
- });
- return r;
- }
//----------------
=======================================
--- /trunk/src/com/google/caja/plugin/taming-membrane.js Tue Oct 8
16:46:28 2013 UTC
+++ /trunk/src/com/google/caja/plugin/taming-membrane.js Mon Oct 14
20:51:59 2013 UTC
@@ -19,20 +19,75 @@
* @overrides window
* @provides TamingMembrane
*/
-// TODO(kpreid): Review privilegedAccess for pieces which are no longer
-// necessary in the post-ES5/3 world. Beware of browser bugs where it
matters
-// which frame the caller belongs to.
-function TamingMembrane(privilegedAccess, schema) {
+function TamingMembrane(helper, schema) {
'use strict';
var feralByTame = new WeakMap();
var tameByFeral = new WeakMap();
- privilegedAccess.weakMapPermitHostObjects(tameByFeral);
+ helper.weakMapPermitHostObjects(tameByFeral);
// Useless value provided as a safe 'this' value to functions.
- feralByTame.set(privilegedAccess.USELESS, privilegedAccess.USELESS);
- tameByFeral.set(privilegedAccess.USELESS, privilegedAccess.USELESS);
+ feralByTame.set(helper.USELESS, helper.USELESS);
+ tameByFeral.set(helper.USELESS, helper.USELESS);
+
+ // Distinguished value returned when directConstructor() wishes to
indicate
+ // that the direct constructor of some object is the native primordial
+ // "Object" function of some JavaScript frame.
+ var BASE_OBJECT_CONSTRUCTOR = Object.freeze({});
+
+ function directConstructor(obj) {
+ if (obj === null) { return void 0; }
+ if (obj === void 0) { return void 0; }
+ if ((typeof obj) !== 'object') {
+ // Regarding functions, since functions return undefined,
+ // directConstructor() doesn't provide access to the
+ // forbidden Function constructor.
+ // Otherwise, we don't support finding the direct constructor
+ // of a primitive.
+ return void 0;
+ }
+ var directProto = Object.getPrototypeOf(obj);
+ if (!directProto) { return void 0; }
+ var directCtor = directProto.constructor;
+ if (!directCtor) { return void 0; }
+ helper.allFrames().forEach(function(w) {
+ var O;
+ try {
+ O = w.Object;
+ } catch (e) {
+ // met a different-origin frame, probably
+ return;
+ }
+ if (directCtor === O) {
+ if
(!Object.prototype.hasOwnProperty.call(directProto, 'constructor')) {
+ // detect prototypes which just didn't bother to
set .constructor and
+ // inherited it from Object (Safari's DOMException is the
motivating
+ // case).
+ directCtor = void 0;
+ } else {
+ directCtor = BASE_OBJECT_CONSTRUCTOR;
+ }
+ }
+ });
+ return directCtor;
+ }
+
+ function getObjectCtorFor(o) {
+ if (o === undefined || o === null) {
+ return void 0;
+ }
+ var ot = typeof o;
+ if (ot !== 'object' && ot !== 'function') {
+ throw new TypeError('Cannot obtain ctor for non-object');
+ }
+ var proto = undefined;
+ while (o) {
+ proto = o;
+ o = Object.getPrototypeOf(o);
+ }
+ return proto.constructor;
+ }
function isNumericName(n) {
return typeof n === 'number' || ('' + (+n)) === n;
@@ -68,8 +123,7 @@
function getFeralProperty(feralObject, feralProp) {
try {
- return tame(
- privilegedAccess.getProperty(feralObject, feralProp));
+ return tame(feralObject[feralProp]);
} catch (e) {
throw tameException(e);
}
@@ -77,7 +131,7 @@
function setFeralProperty(feralObject, feralProp, feralValue) {
try {
- privilegedAccess.setProperty(feralObject, feralProp, feralValue);
+ feralObject[feralProp] = feralValue;
} catch (e) {
throw tameException(e);
}
@@ -107,29 +161,29 @@
var t = void 0;
switch (Object.prototype.toString.call(o)) {
case '[object Boolean]':
- t = new Boolean(privilegedAccess.getValueOf(o));
+ t = new Boolean(o.valueOf());
break;
case '[object Date]':
- t = new Date(privilegedAccess.getValueOf(o));
+ t = new Date(o.valueOf());
break;
case '[object Number]':
- t = new Number(privilegedAccess.getValueOf(o));
+ t = new Number(o.valueOf());
break;
case '[object RegExp]':
t = new RegExp(
- privilegedAccess.getProperty(o, 'source'),
- (privilegedAccess.getProperty(o, 'global') ? 'g' : '') +
- (privilegedAccess.getProperty(o, 'ignoreCase') ? 'i' : '') +
- (privilegedAccess.getProperty(o, 'multiline') ? 'm' : ''));
+ o.source,
+ (o.global ? 'g' : '') +
+ (o.ignoreCase ? 'i' : '') +
+ (o.multiline ? 'm' : ''));
break;
case '[object String]':
- t = new String(privilegedAccess.getValueOf(o));
+ t = new String(o.valueOf());
break;
case '[object Error]':
case '[object DOMException]':
// paranoia -- Error constructor is specified to stringify
- var msg = '' + privilegedAccess.getProperty(o, 'message');
- var name = privilegedAccess.getProperty(o, 'name');
+ var msg = '' + o.message;
+ var name = o.name;
switch (name) {
case 'Error':
t = new Error(msg);
@@ -231,7 +285,7 @@
function tameArray(fa) {
var ta = [];
for (var i = 0; i < fa.length; i++) {
- ta[i] = tame(privilegedAccess.getProperty(fa, i));
+ ta[i] = tame(fa[i]);
}
return Object.freeze(ta);
}
@@ -239,7 +293,7 @@
function untameArray(ta) {
var fa = [];
for (var i = 0; i < ta.length; i++) {
- privilegedAccess.setProperty(fa, i, untame(ta[i]));
+ fa[i] = untame(ta[i]);
}
return Object.freeze(fa);
}
@@ -276,8 +330,8 @@
+ f + '. The membrane has previously been compromised.');
}
if (ftype === 'object') {
- var ctor = privilegedAccess.directConstructor(f);
- if (ctor === privilegedAccess.BASE_OBJECT_CONSTRUCTOR) {
+ var ctor = directConstructor(f);
+ if (ctor === BASE_OBJECT_CONSTRUCTOR) {
t = preventExtensions(tameRecord(f));
} else {
t = copyBuiltin(f);
@@ -322,7 +376,7 @@
function tameRecord(f, t) {
if (!t) { t = {}; }
var readOnly = schema.tameAs.get(f) ===
schema.tameTypes.READ_ONLY_RECORD;
- privilegedAccess.getOwnPropertyNames(f).forEach(function(p) {
+ Object.keys(f).forEach(function(p) {
if (isNumericName(p)) { return; }
if (!isValidPropertyName(p)) { return; }
var get = function() {
@@ -344,7 +398,9 @@
}
function tamePreviouslyConstructedObject(f, fc) {
- if (schema.tameAs.get(fc) !== schema.tameTypes.CONSTRUCTOR) { return
void 0; }
+ if (schema.tameAs.get(fc) !== schema.tameTypes.CONSTRUCTOR) {
+ return void 0;
+ }
var tc = tame(fc);
var t = Object.create(tc.prototype);
tameObjectWithMethods(f, t);
@@ -373,14 +429,17 @@
}
});
}
+
+ // CAUTION: It is ESSENTIAL that we pass USELESS, not (void 0), when
+ // calling down to a feral function. That function may not be declared
+ // in "strict" mode, and so would receive [window] as its "this" arg if
+ // we called it with (void 0), which would be a serious vulnerability.
function tamePureFunction(f) {
var t = function(_) {
- // Since it's by definition useless, there's no reason to bother
- // passing untame(USELESS); we just pass USELESS itself.
return applyFeralFunction(
f,
- privilegedAccess.USELESS,
+ helper.USELESS, // See notes on USELESS above
untameArray(arguments));
};
addFunctionPropertyHandlers(f, t);
@@ -389,14 +448,14 @@
}
function tameCtor(f, fSuper, name) {
- var fPrototype = privilegedAccess.getProperty(f, 'prototype');
+ var fPrototype = f.prototype;
var t = function (_) {
if (!(this instanceof t)) {
// Call as a function
return applyFeralFunction(
f,
- privilegedAccess.USELESS,
+ (void 0),
untameArray(arguments));
} else {
// Call as a constructor
@@ -415,7 +474,7 @@
tameRecord(f, t);
var tPrototype = (function() {
- if (!fSuper || (fSuper ===
privilegedAccess.getObjectCtorFor(fSuper))) {
+ if (!fSuper || (fSuper === getObjectCtorFor(fSuper))) {
return {};
}
if (!schema.tameAs.get(fSuper) === schema.tameTypes.CONSTRUCTOR) {
@@ -493,7 +552,7 @@
var self = this;
return function(_) {
return applyFeralFunction(
- privilegedAccess.getProperty(untame(self), p),
+ untame(self)[p],
untame(self),
untameArray(arguments));
};
@@ -571,12 +630,12 @@
throw new TypeError('Feral object found on tame side of taming
membrane: '
+ t + '. The membrane has previously been compromised.');
}
- if (!privilegedAccess.isDefinedInCajaFrame(t)) {
+ if (!helper.isDefinedInCajaFrame(t)) {
throw new TypeError('Host object leaked without being tamed');
}
if (ttype === 'object') {
- var ctor = privilegedAccess.directConstructor(t);
- if (ctor === privilegedAccess.BASE_OBJECT_CONSTRUCTOR) {
+ var ctor = directConstructor(t);
+ if (ctor === BASE_OBJECT_CONSTRUCTOR) {
f = untameCajaRecord(t);
} else {
f = copyBuiltin(t);
=======================================
--- /trunk/src/com/google/caja/plugin/taming-schema.js Wed Aug 7 17:46:21
2013 UTC
+++ /trunk/src/com/google/caja/plugin/taming-schema.js Mon Oct 14 20:51:59
2013 UTC
@@ -19,7 +19,7 @@
* @overrides window
* @provides TamingSchema
*/
-function TamingSchema(privilegedAccess) {
+function TamingSchema(helper) {
'use strict';
@@ -73,7 +73,7 @@
// All WeakMaps we use deal in host objects, so have a shortcut
function makeWeakMap() {
var map = new WeakMap();
- privilegedAccess.weakMapPermitHostObjects(map);
+ helper.weakMapPermitHostObjects(map);
return map;
}
@@ -108,7 +108,7 @@
if (fixed.has(f)) {
throw new TypeError('Taming controls not for already tamed: ' + f);
}
- if (privilegedAccess.isDefinedInCajaFrame(f)) {
+ if (helper.isDefinedInCajaFrame(f)) {
throw new TypeError('Taming controls not for Caja objects: ' + f);
}
}
@@ -193,7 +193,7 @@
function initAdvice(f) {
if (!functionAdvice.has(f)) {
functionAdvice.set(f, function tamingNullAdvice(self, args) {
- return privilegedAccess.applyFunction(f, self, args);
+ return f.apply(self, args);
});
}
return functionAdvice.get(f);
@@ -202,32 +202,21 @@
function adviseFunctionBefore(f, advice) {
var p = initAdvice(f);
functionAdvice.set(f, function tamingBeforeAdvice(self, args) {
- return p(
- self,
- privilegedAccess.applyFunction(
- advice,
- privilegedAccess.USELESS,
- [f, self, args]));
+ return p(self, advice(f, self, args));
});
}
function adviseFunctionAfter(f, advice) {
var p = initAdvice(f);
functionAdvice.set(f, function tamingAfterAdvice(self, args) {
- return privilegedAccess.applyFunction(
- advice,
- privilegedAccess.USELESS,
- [f, self, p(self, args)]);
+ return advice(f, self, p(self, args));
});
}
function adviseFunctionAround(f, advice) {
var p = initAdvice(f);
functionAdvice.set(f, function tamingAroundAdvice(self, args) {
- return privilegedAccess.applyFunction(
- advice,
- privilegedAccess.USELESS,
- [p, self, args]);
+ return advice(p, self, args);
});
}
--
---
You received this message because you are subscribed to the Google Groups "Google Caja Discuss" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/groups/opt_out.
