Reviewers: MarkM,
Description:
* If a property is an accessor property, traverse the getter rather
than invoking it and using its return value. This avoids possibly
causing side-effects while traversing, and handles accessors on
prototypes which throw if not called on an instance.
* Introduce a new whitelist value, 'accessor'; properties which
are whitelisted as such but not actually accessors are rejected,
as are properties which are accessors but whitelisted as data.
* Check syntax of whitelist as part of register().
* Shorten getThrowTypeError code by using 'gopd'.
Please review this at https://codereview.appspot.com/11778043/
Affected files:
src/com/google/caja/ses/startSES.js
src/com/google/caja/ses/whitelist.js
Index: src/com/google/caja/ses/startSES.js
===================================================================
--- src/com/google/caja/ses/startSES.js (revision 5503)
+++ src/com/google/caja/ses/startSES.js (working copy)
@@ -372,7 +372,7 @@
* Obtain the ES5 singleton [[ThrowTypeError]].
*/
function getThrowTypeError() {
- return
Object.getOwnPropertyDescriptor(getThrowTypeError, "arguments").get;
+ return gopd(getThrowTypeError, "arguments").get;
}
@@ -1384,10 +1384,14 @@
* process "*" inheritance using the whitelist, by walking actual
* superclass chains.
*/
+ var whitelistSymbols = [true, '*', 'accessor'];
var whiteTable = WeakMap();
function register(value, permit) {
if (value !== Object(value)) { return; }
if (typeof permit !== 'object') {
+ if (whitelistSymbols.indexOf(permit) < 0) {
+ fail('syntax error in whitelist; unexpected value: ' + permit);
+ }
return;
}
var oldPermit = whiteTable.get(value);
@@ -1396,8 +1400,13 @@
}
whiteTable.set(value, permit);
keys(permit).forEach(function(name) {
- var sub = value[name];
- register(sub, permit[name]);
+ // Use gopd to avoid invoking an accessor property.
+ // Mismatches between permit === 'accessor' and the property actually
+ // being an accessor property are caught later by clean().
+ var desc = gopd(value, name);
+ if (desc) {
+ register(desc.value, permit[name]);
+ }
});
}
register(sharedImports, whitelist);
@@ -1407,7 +1416,7 @@
* {@code base} object, and if so, with what Permit?
*
* <p>If it should be permitted, return the Permit (where Permit =
- * true | "*" | Record(Permit)), all of which are
+ * true | "accessor" | "*" | Record(Permit)), all of which are
* truthy. If it should not be permitted, return false.
*/
function getPermit(base, name) {
@@ -1572,8 +1581,32 @@
var path = prefix + (prefix ? '.' : '') + name;
var p = getPermit(value, name);
if (p) {
- var sub = value[name];
- clean(sub, path);
+ var desc = gopd(value, name);
+ if (hop.call(desc, 'value')) {
+ // Is a data property
+ var subValue = desc.value;
+ if (p === 'accessor') {
+ // Note this is safe not in that it is safe for the prop to be
an
+ // accessor, but in that cleaning will delete it later (fail
safe).
+ reportProperty(ses.severities.SAFE_SPEC_VIOLATION,
+ 'Not an accessor property', path);
+ cleanProperty(value, name, path);
+ } else {
+ clean(subValue, path);
+ }
+ } else {
+ // Is an accessor property
+ if (p !== 'accessor') {
+ // Note this is safe not in that it is safe for the prop to be
an
+ // accessor, but in that cleaning will delete it later (fail
safe).
+ reportProperty(ses.severities.SAFE_SPEC_VIOLATION,
+ 'Not a data property', path);
+ cleanProperty(value, name, path);
+ } else {
+ clean(desc.get, path + '<getter>');
+ clean(desc.set, path + '<setter>');
+ }
+ }
} else {
cleanProperty(value, name, path);
}
Index: src/com/google/caja/ses/whitelist.js
===================================================================
--- src/com/google/caja/ses/whitelist.js (revision 5503)
+++ src/com/google/caja/ses/whitelist.js (working copy)
@@ -48,6 +48,13 @@
* function) should be further tamed only according to the
* markings of the other objects it inherits from, like {@code
* "Function.prototype"} and {@code "Object.prototype").
+ * <p>If the property is an accessor property, it is not
+ * whitelisted (as invoking an accessor might not be meaningful,
+ * yet the accessor might return a value needing taming).
+ * <li>"accessor", in which case this accessor property is simply
+ * whitelisted and its getter and/or setter are tamed according to
+ * inheritance. If the property is not an accessor property, it is
+ * not whitelisted.
* <li>"*", in which case this property on this object is whitelisted,
* as is this property as inherited by all objects that inherit
* from this object. The values associated with all such properties
--
---
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.
