Revision: 5582
Author: erights
Date: Wed Aug 28 17:45:51 2013 UTC
Log: Protect against alleged-string coercion attacks
https://codereview.appspot.com/13235044
Fixes https://code.google.com/p/google-caja/issues/detail?id=1849
For all the internal verify*(allegedString) methods, change their API
so that they coerce their alleged string to an actual string early,
and then either return that string on success or throw otherwise.
For all exposed evaluators,
cajaVM.{compileExpr, confine, compileModule, eval, Function},
ensure that all of these coerce their untrusted source inputs to
strings early, and thereafter use only these coerced forms.
[email protected]
http://code.google.com/p/google-caja/source/detail?r=5582
Modified:
/trunk/src/com/google/caja/ses/repairES5.js
/trunk/src/com/google/caja/ses/startSES.js
=======================================
--- /trunk/src/com/google/caja/ses/repairES5.js Tue Aug 27 23:54:54 2013 UTC
+++ /trunk/src/com/google/caja/ses/repairES5.js Wed Aug 28 17:45:51 2013 UTC
@@ -669,6 +669,13 @@
*
* <p>See verifyStrictFunctionBodyByEvalThrowing and
* verifyStrictFunctionBodyByParsing.
+ *
+ * <p>Note that all verify*(allegedString) functions now always
+ * start by coercing the alleged string to a guaranteed primitive
+ * string, do their verification checks on that, and if it passes,
+ * returns that. Otherwise they throw. If you don't know whether
+ * something is a string before verifying, use only the output of
+ * the verifier, not the input. Or coerce it early yourself.
*/
ses.verifyStrictFunctionBody = simpleVerifyStrictFunctionBody;
@@ -687,7 +694,9 @@
* SyntaxError if it does not parse as a FunctionBody.
*/
function simpleVerifyStrictFunctionBody(funcBodySrc) {
+ funcBodySrc = ''+funcBodySrc;
UnsafeFunction('"use strict";' + funcBodySrc);
+ return funcBodySrc;
}
/**
@@ -707,6 +716,7 @@
* (simpleVerifyStrictFunctionBody) executed over 100x faster on V8.
*/
function verifyStrictFunctionBodyByEvalThrowing(funcBodySrc) {
+ funcBodySrc = ''+funcBodySrc;
try {
unsafeEval('"use strict"; throw "not a SyntaxError 1";' +
'(function(){' + funcBodySrc +'\n});');
@@ -724,7 +734,7 @@
// function declarations will not get rejected. Accepting
// them is beyond the ES5 spec, but is known to happen in
// all implementations.
- return;
+ return funcBodySrc;
}
if (innerErr instanceof SyntaxError) {
// This case is likely symptomatic of an attack. But the
@@ -765,6 +775,7 @@
* should overwrite ses.mitigateSrcGotchas with the real one.
*/
function verifyStrictFunctionBodyByParsing(funcBodySrc) {
+ funcBodySrc = ''+funcBodySrc;
var safeError;
var newSrc;
try {
@@ -800,6 +811,7 @@
if (newSrc !== funcBodySrc) {
throw new SyntaxError('Failed to parse program');
}
+ return funcBodySrc;
}
/**
=======================================
--- /trunk/src/com/google/caja/ses/startSES.js Wed Aug 7 22:47:37 2013 UTC
+++ /trunk/src/com/google/caja/ses/startSES.js Wed Aug 28 17:45:51 2013 UTC
@@ -555,10 +555,19 @@
* no exprSrc text which can close the left paren(s), do
* something, and then provide open paren(s) to balance the final
* close paren(s). No one such attack will survive both tests.
+ *
+ * <p>Note that all verify*(allegedString) functions now always
+ * start by coercing the alleged string to a guaranteed primitive
+ * string, do their verification checks on that, and if it passes,
+ * returns that. Otherwise they throw. If you don't know whether
+ * something is a string before verifying, use only the output of
+ * the verifier, not the input. Or coerce it early yourself.
*/
function verifyStrictExpression(exprSrc) {
+ exprSrc = ''+exprSrc;
ses.verifyStrictFunctionBody('( ' + exprSrc + '\n);');
ses.verifyStrictFunctionBody('(( ' + exprSrc + '\n));');
+ return exprSrc;
}
/**
@@ -726,7 +735,7 @@
* </ul>
*/
function securableWrapperSrc(exprSrc) {
- verifyStrictExpression(exprSrc);
+ exprSrc = verifyStrictExpression(exprSrc);
return '(function() { ' +
// non-strict code, where this === scopeObject
@@ -823,9 +832,10 @@
* {@code with} together with RegExp matching to intercept free
* variable access without parsing.
*/
- function compileExpr(src, opt_mitigateOpts) {
- // Force src to be parsed as an expr
- var exprSrc = '(' + src + '\n)';
+ function compileExpr(exprSrc, opt_mitigateOpts) {
+ // Force exprSrc to be a string that can only parse (if at all) as
+ // an expression.
+ exprSrc = '(' + exprSrc + '\n)';
var options = resolveOptions(opt_mitigateOpts);
exprSrc = mitigateSrcGotchas(exprSrc, options);
@@ -854,6 +864,11 @@
* expr to cause effects.
*/
function confine(exprSrc, opt_endowments, opt_mitigateOpts) {
+ // not necessary, since we only use it once below with a callee
+ // which is itself safe. But we coerce to a string anyway to be
+ // more robust against future refactorings.
+ exprSrc = ''+exprSrc;
+
var imports = makeImports();
if (opt_endowments) {
copyToImports(imports, opt_endowments);
@@ -925,12 +940,15 @@
* from the text to be compiled.
*/
function compileModule(modSrc, opt_mitigateOpts) {
- // Note the EOL after modSrc to prevent trailing line comment in
modSrc
- // eliding the rest of the wrapper.
+ // See https://code.google.com/p/google-caja/issues/detail?id=1849
+ modSrc = ''+modSrc;
+
var options = resolveOptions(opt_mitigateOpts);
if (!('programSrc' in limitSrcCharset(modSrc))) {
options.forceParseAndRender = true;
}
+ // Note the EOL after modSrc to prevent a trailing line comment in
+ // modSrc from eliding the rest of the wrapper.
var exprSrc =
'(function() {' +
mitigateSrcGotchas(modSrc, options) +
@@ -955,12 +973,16 @@
*/
function FakeFunction(var_args) {
var params = [].slice.call(arguments, 0);
- var body = params.pop();
- body = String(body || '');
+ var body = ses.verifyStrictFunctionBody(params.pop() || '');
+
+ // Although the individual params may not be strings, the params
+ // array is reliably a fresh array, so under the SES (not CES)
+ // assumptions of unmodified primordials, this calls the reliable
+ // Array.prototype.join which guarantees that its result is a string.
params = params.join(',');
- // Note the EOL after modSrc to prevent trailing line comment in body
- // eliding the rest of the wrapper.
- ses.verifyStrictFunctionBody(body);
+
+ // Note the EOL after body to prevent a trailing line comment in
+ // body from eliding the rest of the wrapper.
var exprSrc = '(function(' + params + '\n){' + body + '\n})';
return compileExpr(exprSrc)(sharedImports);
}
@@ -996,7 +1018,7 @@
*/
function fakeEval(src) {
try {
- verifyStrictExpression(src);
+ src = verifyStrictExpression(src);
} catch (x) {
src = '(function() {' + src + '\n}).call(this)';
}
--
---
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.
