btw this is making firefox 22 (beta) fail. haven't looked at why yet On Fri, Jun 7, 2013 at 2:10 PM, <[email protected]> wrote: > Revision: 5442 > Author: [email protected] > Date: Fri Jun 7 14:10:14 2013 > Log: [APICHANGE] Replace NO_KNOWN_EXPLOIT_SPEC_VIOLATION with > configuration. > https://codereview.appspot.com/9979047 > > The notion of NO_KNOWN_EXPLOIT_SPEC_VIOLATION (NKESV) depends on the use > being made of SES (for example, the cross-frame freeze bug is > problematic only if you use multiple frames). Therefore, it should not > be hardcoded in SES itself, but supplied by the system loading SES. > > * Remove NKESV and replace it with ses.acceptableProblems, which > allows specifying 'permit' and 'doNotRepair' flags for problem IDs. > > 'permit' causes a problem's severity to not be counted in the max > severity. 'doNotRepair' causes a problem to not be repaired at all; > this new feature may be used to improve performance by disabling slow > repairs that are not necessary for the application. > > * caja.js specially recognizes NKESV in its config and causes acceptance > of the same list of problems to be accepted as NKESV did. For safety, > and pending design, there is no way to configure the list of problems > using caja.js. > > * Replace ses-iframe-init.js with a hook which runs on an iframe before > the ses-single-frame.js is loaded; this also allows us to throw out > the kludge where we set SES's max severity to NEW_SYMPTOM and check > separately after (which incidentally reduces the hazard of > issue 1758), and might make autoswitching faster. > > * Resurrect the "too slow" repair_PUSH_IGNORES_SEALED from r5238 and > disable it using doNotRepair. Note that the repair will now be applied > in Caja unless NKESV is requested. > > Impact: > * Applications using SES directly must specify acceptableProblems rather > than NKESV. Applications using caja.js are unaffected except that they > will now get repair_PUSH_IGNORES_SEALED unless specifying NKESV. > > [email protected], [email protected] > > http://code.google.com/p/google-caja/source/detail?r=5442 > > Deleted: > /trunk/src/com/google/caja/plugin/ses-iframe-init.js > Modified: > /trunk/build.xml > /trunk/src/com/google/caja/plugin/caja.js > /trunk/src/com/google/caja/ses/repairES5.js > > ======================================= > --- /trunk/src/com/google/caja/plugin/ses-iframe-init.js Tue Aug 16 > 16:44:30 2011 > +++ /dev/null > @@ -1,35 +0,0 @@ > -// Copyright (C) 2011 Google Inc. > -// > -// Licensed under the Apache License, Version 2.0 (the "License"); > -// you may not use this file except in compliance with the License. > -// You may obtain a copy of the License at > -// > -// http://www.apache.org/licenses/LICENSE-2.0 > -// > -// Unless required by applicable law or agreed to in writing, software > -// distributed under the License is distributed on an "AS IS" BASIS, > -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. > -// See the License for the specific language governing permissions and > -// limitations under the License. > - > -/** > - * @fileoverview Configure initSES to operate as expected by caja.js > - * taming and guest frames. > - * > - * In particular, we disable the maximum-problem-severity check, because > - * caja.js will check the security itself and we want our code to > - * not fail partway through loading. > - * > - * TODO(kpreid): This strategy is insufficient to deal with non-ES5-capable > - * browsers, because we will crash partway through loading Domado or its > deps. > - * Instead, we need to wrap code-after-initSES in a function and not run it > - * if SES failed, or load a separate frame to probe SES status in, or load > - * initSES with a callback and then load Domado etc. afterward. > - * > - * @author [email protected] (Kevin Reid) > - * @overrides ses > - */ > - > -var ses; > -if (!ses) { ses = {}; } > -ses.maxAcceptableSeverityName = 'NEW_SYMPTOM'; > ======================================= > --- /trunk/build.xml Wed May 29 22:59:13 2013 > +++ /trunk/build.xml Fri Jun 7 14:10:14 2013 > @@ -1072,7 +1072,6 @@ > language="javascript" renderer="concat"/> > <input file="${src.caja}/plugin/caja-iframe-build-version.js"/> > <input file="${third_party}/js/json_sans_eval/json_sans_eval.js"/> > - <input file="${src.caja}/plugin/ses-iframe-init.js"/> > <input file="${lib.caja}/ses/initSESPlus.js" jslint="false"/> > <input file="${src.caja}/plugin/ses-iframe-earlyfail.js"/> > > ======================================= > --- /trunk/src/com/google/caja/plugin/caja.js Wed Jun 5 20:35:14 2013 > +++ /trunk/src/com/google/caja/plugin/caja.js Fri Jun 7 14:10:14 2013 > @@ -358,8 +358,16 @@ > } else { > full['es5Mode'] = > partial['es5Mode'] === undefined ? GUESS : !!partial['es5Mode']; > - full['maxAcceptableSeverity'] = > - String(partial['maxAcceptableSeverity'] || 'SAFE_SPEC_VIOLATION'); > + var severity = String(partial['maxAcceptableSeverity'] || > + 'SAFE_SPEC_VIOLATION'); > + if (severity === 'NO_KNOWN_EXPLOIT_SPEC_VIOLATION') { > + // Special severity level which SES itself no longer implements > + // TODO(kpreid): Should acceptNoKnownExploitProblems be part of our > + // public interface? > + severity = 'SAFE_SPEC_VIOLATION'; > + full['acceptNoKnownExploitProblems'] = true; > + } > + full['maxAcceptableSeverity'] = severity; > } > > if (partial['console']) { > @@ -427,13 +435,35 @@ > } > > function trySES(config, frameGroupReady, onFailure) { > - var sesMaker = makeFrameMaker(config, 'ses-single-frame'); > + function frameInit(frameWin) { > + var ses = frameWin['ses'] || (frameWin['ses'] = {}); > + ses['maxAcceptableSeverityName'] = config['maxAcceptableSeverity']; > + if (config['acceptNoKnownExploitProblems']) { > + ses['acceptableProblems'] = { > + 'DEFINING_READ_ONLY_PROTO_FAILS_SILENTLY': { 'permit': true }, > + > + // we don't use partly-unmodifiable arrays, and the repair for > push > + // is too slow > + 'PUSH_IGNORES_SEALED': { 'permit': true, 'doNotRepair': true }, > + 'PUSH_IGNORES_FROZEN': { 'doNotRepair': true }, > + 'PUSH_DOES_NOT_THROW_ON_FROZEN_ARRAY': > + { 'permit': true, 'doNotRepair': true }, > + 'ARRAYS_DELETE_NONCONFIGURABLE': { 'permit': true }, > + 'ARRAYS_MODIFY_READONLY': { 'permit': true }, > + > + // safe given that we use exactly one SES frame > + 'FREEZE_IS_FRAME_DEPENDENT': { 'permit': true } > + }; > + } > + } > + > + var sesMaker = makeFrameMaker(config, 'ses-single-frame', frameInit); > > loadCajaFrame(config, 'utility-frame', function (mitigateWin) { > var mitigateSrcGotchas = mitigateWin['ses']['mitigateSrcGotchas']; > sesMaker['make'](function (tamingWin) { > var mustSES = config['es5Mode'] === true; > - if (canSES(tamingWin['ses'], config['maxAcceptableSeverity'])) { > + if (tamingWin['ses']['ok']()) { > var fg = tamingWin['SESFrameGroup']( > cajaInt, config, tamingWin, window, > { 'mitigateSrcGotchas': mitigateSrcGotchas }); > @@ -453,10 +483,6 @@ > }); > }); > } > - > - function canSES(ses, severity) { > - return ses['ok'](ses['severities'][severity]); > - } > > // Fast rejection of SES. If this works, repairES5 might still fail, and > // we'll fall back to ES53 then. > @@ -471,7 +497,7 @@ > * Calling frameMaker.preload() will start creation of a new frame now, > * and make it available to a later call to frameMaker.make(). > */ > - function makeFrameMaker(config, filename) { > + function makeFrameMaker(config, filename, opt_frameCreated) { > var IDLE = 'IDLE', LOADING = 'LOADING', WAITING = 'WAITING'; > var preState = IDLE, preWin, preReady; > var self = { > @@ -482,7 +508,7 @@ > loadCajaFrame(config, filename, function (win) { > preWin = win; > consumeIfReady(); > - }); > + }, opt_frameCreated); > } > }, > 'make': function (onReady) { > @@ -491,7 +517,7 @@ > preReady = onReady; > consumeIfReady(); > } else { > - loadCajaFrame(config, filename, onReady); > + loadCajaFrame(config, filename, onReady, opt_frameCreated); > } > } > }; > @@ -511,7 +537,7 @@ > > //---------------- > > - function loadCajaFrame(config, filename, frameReady) { > + function loadCajaFrame(config, filename, frameReady, opt_frameCreated) { > var frameWin = createFrame(filename); > // debuggable or minified. ?debug=1 inhibits compilation in shindig > var suffix = config['debug'] ? '.js?debug=1' : '.opt.js?debug=1'; > @@ -526,6 +552,7 @@ > versionCheck(config, frameWin, filename); > frameReady(frameWin); > }; > + if (opt_frameCreated) { opt_frameCreated(frameWin); } > // TODO(jasvir): Test what the latency doing this on all browsers is > // and why its necessary > setTimeout(function () { > ======================================= > --- /trunk/src/com/google/caja/ses/repairES5.js Fri May 31 15:06:53 2013 > +++ /trunk/src/com/google/caja/ses/repairES5.js Fri Jun 7 14:10:14 2013 > @@ -31,6 +31,7 @@ > * //provides ses.makeCallerHarmless, ses.makeArgumentsHarmless > * //provides ses.severities, ses.maxSeverity, ses.updateMaxSeverity > * //provides ses.maxAcceptableSeverityName, ses.maxAcceptableSeverity > + * //provides ses.acceptableProblems > * > * @author Mark S. Miller > * @requires ___global_test_function___, ___global_valueOf_function___ > @@ -96,9 +97,6 @@ > * <dt>SAFE_SPEC_VIOLATION</dt> > * <dd>safe (in an integrity sense) even if unrepaired. May > * still lead to inappropriate failures.</dd> > - * <dt>NO_KNOWN_EXPLOIT_SPEC_VIOLATION</dt> > - * <dd>known to introduce an indirect safety issue which, > - * however, is not known to be exploitable.</dd> > * <dt>UNSAFE_SPEC_VIOLATION</dt> > * <dd>a safety issue only indirectly, in that this spec > * violation may lead to the corruption of assumptions made > @@ -120,8 +118,6 @@ > MAGICAL_UNICORN: { level: -1, description: 'Testing only' }, > SAFE: { level: 0, description: 'Safe' }, > SAFE_SPEC_VIOLATION: { level: 1, description: 'Safe spec violation' > }, > - NO_KNOWN_EXPLOIT_SPEC_VIOLATION: { > - level: 2, description: 'Unsafe spec violation but no known > exploits' }, > UNSAFE_SPEC_VIOLATION: { level: 3, description: 'Unsafe spec violation' > }, > NOT_OCAP_SAFE: { level: 4, description: 'Not ocap safe' }, > NOT_ISOLATED: { level: 5, description: 'Not isolated' }, > @@ -139,6 +135,9 @@ > * <dd>test failed before and after repair attempt.</dd> > * <dt>NOT_REPAIRED</dt> > * <dd>test failed before and after, with no repair to attempt.</dd> > + * <dt>REPAIR_SKIPPED</dt> > + * <dd>test failed before and after, and ses.acceptableProblems > + * specified not to repair it.</dd> > * <dt>REPAIRED_UNSAFELY</dt> > * <dd>test failed before and passed after repair attempt, but > * the repair is known to be inadequate for security, so the > @@ -159,6 +158,7 @@ > ALL_FINE: 'All fine', > REPAIR_FAILED: 'Repair failed', > NOT_REPAIRED: 'Not repaired', > + REPAIR_SKIPPED: 'Repair skipped', > REPAIRED_UNSAFELY: 'Repaired unsafely', > REPAIRED: 'Repaired', > ACCIDENTALLY_REPAIRED: 'Accidentally repaired', > @@ -207,6 +207,9 @@ > * have the choice of using SES on those platforms which we judge to > * be adequately repairable, or otherwise falling back to Caja's > * ES5/3 translator. > + * > + * <p>See also {@code ses.acceptableProblems} for overriding the > + * severity of specific known problems. > */ > ses.maxAcceptableSeverityName = > validateSeverityName(ses.maxAcceptableSeverityName); > @@ -232,6 +235,62 @@ > function severityNameToLevel(severityName) { > return ses.severities[validateSeverityName(severityName)]; > } > + > + /** > + * An object whose enumerable keys are problem names and whose values > + * are records containing the following boolean properties, defaulting > + * to false if omitted: > + * <dl> > + * > + * <dt>{@code permit} > + * <dd>If this problem is not repaired, continue even if its severity > + * would otherwise be too great (maxSeverity will be as if this > + * problem does not exist). Use this for problems which are known > + * to be acceptable for the particular use case of SES. > + * > + * <p>THIS CONFIGURATION IS POTENTIALLY EXTREMELY DANGEROUS. Ignoring > + * problems can make SES itself insecure in subtle ways even if you > + * do not use any of the affected features in your own code. Do not > + * use it without full understanding of the implications. > + * > + * <p>TODO(kpreid): Add a flag to kludge records to indicate whether > + * the problems may be ignored and check it here. > + * </dd> > + * > + * <dt>{@code doNotRepair} > + * <dd>Do not attempt to repair this problem. > + * Use this for problems whose repairs have unacceptable disadvantages. > + * > + * <p>Observe that if {@code permit} is also false, then this means to > + * abort rather than repairing, whereas if {@code permit} is true then > + * this means to continue without repairing the problem even if it is > + * repairable. > + * > + * </dl> > + */ > + ses.acceptableProblems = > validateAcceptableProblems(ses.acceptableProblems); > + > + function validateAcceptableProblems(opt_problems) { > + var validated = {}; > + if (opt_problems) { > + for (var problem in opt_problems) { > + // TODO(kpreid): Validate problem names. > + var flags = opt_problems[problem]; > + if (typeof flags !== 'object') { > + throw new Error('ses.acceptableProblems["' + problem + '"] is > not' + > + ' an object, but ' + flags); > + } > + var valFlags = {permit: false, doNotRepair: false}; > + for (var flag in flags) { > + if (valFlags.hasOwnProperty(flag)) { > + valFlags[flag] = Boolean(flags[flag]); > + } > + } > + validated[problem] = valFlags; > + } > + } > + return validated; > + } > > /** > * Once this returns false, we can give up on the SES > @@ -2962,6 +3021,21 @@ > writable: true > }); > } > + > + function repair_PUSH_IGNORES_SEALED() { > + var push = Array.prototype.push; > + var sealed = Object.isSealed; > + Object.defineProperty(Array.prototype, 'push', { > + value: function(compareFn) { > + if (sealed(this)) { > + throw new TypeError('Cannot push onto a sealed object.'); > + } > + return push.apply(this, arguments); > + }, > + configurable: true, > + writable: true > + }); > + } > > // error message is matched elsewhere (for tighter bounds on catch) > var NO_CREATE_NULL = > @@ -3590,7 +3664,7 @@ > description: 'Defining __proto__ on non-extensible object fails > silently', > test: test_DEFINING_READ_ONLY_PROTO_FAILS_SILENTLY, > repair: repair_DEFINE_PROPERTY, > - preSeverity: severities.NO_KNOWN_EXPLOIT_SPEC_VIOLATION, > + preSeverity: severities.UNSAFE_SPEC_VIOLATION, > canRepair: true, > urls: ['http://code.google.com/p/v8/issues/detail?id=2441'], > sections: ['8.6.2'], > @@ -3683,9 +3757,9 @@ > id: 'PUSH_IGNORES_SEALED', > description: 'Array.prototype.push ignores sealing', > test: test_PUSH_IGNORES_SEALED, > - repair: undefined, // workaround is too slow > - preSeverity: severities.NO_KNOWN_EXPLOIT_SPEC_VIOLATION, > - canRepair: false, > + repair: repair_PUSH_IGNORES_SEALED, > + preSeverity: severities.UNSAFE_SPEC_VIOLATION, > + canRepair: true, > urls: ['http://code.google.com/p/v8/issues/detail?id=2412'], > sections: ['15.4.4.11'], > tests: [] // TODO(erights): Add to test262 > @@ -3694,9 +3768,9 @@ > id: 'PUSH_DOES_NOT_THROW_ON_FROZEN_ARRAY', > description: 'Array.prototype.push does not throw on a frozen array', > test: test_PUSH_DOES_NOT_THROW_ON_FROZEN_ARRAY, > - repair: undefined, > - preSeverity: severities.NO_KNOWN_EXPLOIT_SPEC_VIOLATION, > - canRepair: false, > + repair: repair_PUSH_IGNORES_SEALED, > + preSeverity: severities.UNSAFE_SPEC_VIOLATION, > + canRepair: true, > urls: [], > sections: ['15.2.3.9'], > tests: [] // TODO(erights): Add to test262 > @@ -3705,9 +3779,9 @@ > id: 'PUSH_IGNORES_FROZEN', > description: 'Array.prototype.push ignores frozen', > test: test_PUSH_IGNORES_FROZEN, > - repair: undefined, > + repair: repair_PUSH_IGNORES_SEALED, > preSeverity: severities.UNSAFE_SPEC_VIOLATION, > - canRepair: false, > + canRepair: true, > urls: [], > sections: ['15.2.3.9'], > tests: [] // TODO(erights): Add to test262 > @@ -3717,7 +3791,7 @@ > description: 'Setting [].length can delete non-configurable > elements', > test: test_ARRAYS_DELETE_NONCONFIGURABLE, > repair: void 0, > - preSeverity: severities.NO_KNOWN_EXPLOIT_SPEC_VIOLATION, > + preSeverity: severities.UNSAFE_SPEC_VIOLATION, > canRepair: false, > urls: ['https://bugzilla.mozilla.org/show_bug.cgi?id=590690'], > sections: ['15.4.5.2'], > @@ -3728,7 +3802,7 @@ > description: 'Extending an array can modify read-only array length', > test: test_ARRAYS_MODIFY_READONLY, > repair: void 0, > - preSeverity: severities.NO_KNOWN_EXPLOIT_SPEC_VIOLATION, > + preSeverity: severities.UNSAFE_SPEC_VIOLATION, > canRepair: false, > urls: ['http://code.google.com/p/v8/issues/detail?id=2379'], > sections: ['15.4.5.1.3.f'], > @@ -3750,8 +3824,7 @@ > description: 'Object.freeze falsely succeeds on other-frame objects', > test: test_FREEZE_IS_FRAME_DEPENDENT, > repair: repair_FREEZE_IS_FRAME_DEPENDENT, > - // NO_KNOWN_EXPLOITness depends on using exactly one SES frame > - preSeverity: severities.NO_KNOWN_EXPLOIT_SPEC_VIOLATION, > + preSeverity: severities.UNSAFE_SPEC_VIOLATION, > canRepair: false, // repair is useful but inadequate > urls: ['https://bugzilla.mozilla.org/show_bug.cgi?id=784892', > 'https://bugzilla.mozilla.org/show_bug.cgi?id=674195'], > @@ -3868,6 +3941,12 @@ > ////////////////////// Testing, Repairing, Reporting /////////// > > var aboutTo = void 0; > + > + var defaultDisposition = { permit: false, doNotRepair: false }; > + function disposition(kludge) { > + return ses.acceptableProblems.hasOwnProperty(kludge.id) > + ? ses.acceptableProblems[kludge.id] : defaultDisposition; > + } > > /** > * Run a set of tests & repairs, and report results. > @@ -3885,7 +3964,7 @@ > }); > var repairs = []; > strictForEachFn(kludges, function(kludge, i) { > - if (beforeFailures[i]) { > + if (beforeFailures[i] && !disposition(kludge).doNotRepair) { > var repair = kludge.repair; > if (repair && repairs.lastIndexOf(repair) === -1) { > aboutTo = ['repair: ', kludge.description]; > @@ -3913,7 +3992,10 @@ > var afterFailure = afterFailures[i]; > if (beforeFailure) { // failed before > if (afterFailure) { // failed after > - if (kludge.repair) { > + if (disposition(kludge).doNotRepair) { > + postSeverity = kludge.preSeverity; > + status = statuses.REPAIR_SKIPPED; > + } else if (kludge.repair) { > postSeverity = kludge.preSeverity; > status = statuses.REPAIR_FAILED; > } else { > @@ -3923,7 +4005,7 @@ > status = statuses.NOT_REPAIRED; > } > } else { // succeeded after > - if (kludge.repair) { > + if (kludge.repair && !disposition(kludge).doNotRepair) { > if (!kludge.canRepair) { > // repair for development, not safety > postSeverity = kludge.preSeverity; > @@ -3955,7 +4037,12 @@ > postSeverity = severities.NEW_SYMPTOM; > } > > - ses.updateMaxSeverity(postSeverity); > + if (postSeverity !== severities.SAFE && disposition(kludge).permit) { > + logger.warn('Problem ignored by configuration (' + > + postSeverity.description + '): ' + kludge.description); > + } else { > + ses.updateMaxSeverity(postSeverity); > + } > > return { > id: kludge.id, > > -- > > ---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. > >
-- --- 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.
