Reject (JSON is fixed now) E4X masquerading as JS source (r=igor/mrbkap).
authorBrendan Eich <brendan@mozilla.org>
Fri, 15 May 2009 13:38:19 -0400
changeset 25220 c4ba70070012
parent 25219 872f676e5d1a
child 25221 067280c078d0
push id1425
push userrsayre@mozilla.com
push date2009-05-15 18:15 +0000
reviewersigor, mrbkap
milestone1.9.1b5pre
Reject (JSON is fixed now) E4X masquerading as JS source (r=igor/mrbkap).
dom/src/base/nsJSEnvironment.cpp
js/src/js.msg
js/src/jsparse.cpp
js/src/jsscan.cpp
js/src/jsscan.h
--- a/dom/src/base/nsJSEnvironment.cpp
+++ b/dom/src/base/nsJSEnvironment.cpp
@@ -1520,17 +1520,19 @@ nsJSContext::EvaluateString(const nsAStr
                             PRUint32 aLineNo,
                             PRUint32 aVersion,
                             nsAString *aRetValue,
                             PRBool* aIsUndefined)
 {
   NS_ENSURE_TRUE(mIsInitialized, NS_ERROR_NOT_INITIALIZED);
 
   if (!mScriptsEnabled) {
-    *aIsUndefined = PR_TRUE;
+    if (aIsUndefined) {
+      *aIsUndefined = PR_TRUE;
+    }
 
     if (aRetValue) {
       aRetValue->Truncate();
     }
 
     return NS_OK;
   }
 
@@ -1589,23 +1591,23 @@ nsJSContext::EvaluateString(const nsAStr
   // SecurityManager said "ok", but don't compile if aVersion is unknown.
   // Since the caller is responsible for parsing the version strings, we just
   // check it isn't JSVERSION_UNKNOWN.
   if (ok && ((JSVersion)aVersion) != JSVERSION_UNKNOWN) {
     JSAutoRequest ar(mContext);
     nsJSVersionSetter setVersion(mContext, aVersion);
 
     ok = ::JS_EvaluateUCScriptForPrincipals(mContext,
-                                              (JSObject *)aScopeObject,
-                                              jsprin,
-                                              (jschar*)PromiseFlatString(aScript).get(),
-                                              aScript.Length(),
-                                              aURL,
-                                              aLineNo,
-                                              vp);
+                                            (JSObject *)aScopeObject,
+                                            jsprin,
+                                            (jschar*)PromiseFlatString(aScript).get(),
+                                            aScript.Length(),
+                                            aURL,
+                                            aLineNo,
+                                            vp);
 
     if (!ok) {
       // Tell XPConnect about any pending exceptions. This is needed
       // to avoid dropping JS exceptions in case we got here through
       // nested calls through XPConnect.
 
       JS_ReportPendingException(mContext);
     }
--- a/js/src/js.msg
+++ b/js/src/js.msg
@@ -153,17 +153,17 @@ MSG_DEF(JSMSG_MISSING_FORMAL,          7
 MSG_DEF(JSMSG_PAREN_AFTER_FORMAL,      71, 0, JSEXN_SYNTAXERR, "missing ) after formal parameters")
 MSG_DEF(JSMSG_CURLY_BEFORE_BODY,       72, 0, JSEXN_SYNTAXERR, "missing { before function body")
 MSG_DEF(JSMSG_CURLY_AFTER_BODY,        73, 0, JSEXN_SYNTAXERR, "missing } after function body")
 MSG_DEF(JSMSG_PAREN_BEFORE_COND,       74, 0, JSEXN_SYNTAXERR, "missing ( before condition")
 MSG_DEF(JSMSG_PAREN_AFTER_COND,        75, 0, JSEXN_SYNTAXERR, "missing ) after condition")
 MSG_DEF(JSMSG_DESTRUCT_DUP_ARG,        76, 0, JSEXN_SYNTAXERR, "duplicate argument is mixed with destructuring pattern")
 MSG_DEF(JSMSG_NAME_AFTER_DOT,          77, 0, JSEXN_SYNTAXERR, "missing name after . operator")
 MSG_DEF(JSMSG_BRACKET_IN_INDEX,        78, 0, JSEXN_SYNTAXERR, "missing ] in index expression")
-MSG_DEF(JSMSG_UNUSED79,                79, 0, JSEXN_NONE, "unused79")
+MSG_DEF(JSMSG_XML_WHOLE_PROGRAM,       79, 0, JSEXN_SYNTAXERR, "XML cannot be the whole program")
 MSG_DEF(JSMSG_PAREN_BEFORE_SWITCH,     80, 0, JSEXN_SYNTAXERR, "missing ( before switch expression")
 MSG_DEF(JSMSG_PAREN_AFTER_SWITCH,      81, 0, JSEXN_SYNTAXERR, "missing ) after switch expression")
 MSG_DEF(JSMSG_CURLY_BEFORE_SWITCH,     82, 0, JSEXN_SYNTAXERR, "missing { before switch body")
 MSG_DEF(JSMSG_COLON_AFTER_CASE,        83, 0, JSEXN_SYNTAXERR, "missing : after case label")
 MSG_DEF(JSMSG_WHILE_AFTER_DO,          84, 0, JSEXN_SYNTAXERR, "missing while after do-loop body")
 MSG_DEF(JSMSG_PAREN_AFTER_FOR,         85, 0, JSEXN_SYNTAXERR, "missing ( after for")
 MSG_DEF(JSMSG_SEMI_AFTER_FOR_INIT,     86, 0, JSEXN_SYNTAXERR, "missing ; after for-loop initializer")
 MSG_DEF(JSMSG_SEMI_AFTER_FOR_COND,     87, 0, JSEXN_SYNTAXERR, "missing ; after for-loop condition")
--- a/js/src/jsparse.cpp
+++ b/js/src/jsparse.cpp
@@ -849,16 +849,21 @@ JSCompiler::compileScript(JSContext *cx,
      */
     uint32 bodyid;
     if (!GenerateBlockId(&cg, bodyid))
         return NULL;
     cg.bodyid = bodyid;
 
     /* Null script early in case of error, to reduce our code footprint. */
     script = NULL;
+#if JS_HAS_XML_SUPPORT
+    pn = NULL;
+    bool onlyXML = true;
+#endif
+
     for (;;) {
         jsc.tokenStream.flags |= TSF_OPERAND;
         tt = js_PeekToken(cx, &jsc.tokenStream);
         jsc.tokenStream.flags &= ~TSF_OPERAND;
         if (tt <= TOK_EOF) {
             if (tt == TOK_EOF)
                 break;
             JS_ASSERT(tt == TOK_ERROR);
@@ -876,19 +881,40 @@ JSCompiler::compileScript(JSContext *cx,
         if (cg.functionList) {
             if (!jsc.analyzeFunctions(cg.functionList, cg.flags))
                 goto out;
             cg.functionList = NULL;
         }
 
         if (!js_EmitTree(cx, &cg, pn))
             goto out;
+#if JS_HAS_XML_SUPPORT
+        if (PN_TYPE(pn) != TOK_SEMI ||
+            !pn->pn_kid ||
+            !TREE_TYPE_IS_XML(PN_TYPE(pn->pn_kid))) {
+            onlyXML = false;
+        }
+#endif
         RecycleTree(pn, &cg);
     }
 
+#if JS_HAS_XML_SUPPORT
+    /*
+     * Prevent XML data theft via <script src="http://victim.com/foo.xml">.
+     * For background, see:
+     *
+     * https://bugzilla.mozilla.org/show_bug.cgi?id=336551
+     */
+    if (pn && onlyXML && (tcflags & TCF_NO_SCRIPT_RVAL)) {
+        js_ReportCompileErrorNumber(cx, &jsc.tokenStream, NULL, JSREPORT_ERROR,
+                                    JSMSG_XML_WHOLE_PROGRAM);
+        goto out;
+    }
+#endif
+
     /*
      * Global variables and regexps share the index space with locals. Due to
      * incremental code generation we need to patch the bytecode to adjust the
      * local references to skip the globals.
      */
     scriptGlobals = cg.ngvars + cg.regexpList.length;
     if (scriptGlobals != 0) {
         jsbytecode *code, *end;
--- a/js/src/jsscan.cpp
+++ b/js/src/jsscan.cpp
@@ -1554,20 +1554,29 @@ retry:
          *
          * Which means you will get <!-- comments to end of line in the middle
          * of .js files, and after if conditions whose then statements are on
          * the next line, and other wonders.  See at least the following bugs:
          * https://bugzilla.mozilla.org/show_bug.cgi?id=309242
          * https://bugzilla.mozilla.org/show_bug.cgi?id=309712
          * https://bugzilla.mozilla.org/show_bug.cgi?id=310993
          *
-         * So without JSOPTION_XML, we never scan an XML comment or CDATA
-         * literal.  We always scan <! as the start of an HTML comment hack
-         * to end of line, used since Netscape 2 to hide script tag content
-         * from script-unaware browsers.
+         * So without JSOPTION_XML, we changed around Firefox 1.5 never to scan
+         * an XML comment or CDATA literal.  Instead, we always scan <! as the
+         * start of an HTML comment hack to end of line, used since Netscape 2
+         * to hide script tag content from script-unaware browsers.
+         *
+         * But this still leaves XML resources with certain internal structure
+         * vulnerable to being loaded as script cross-origin, and some internal
+         * data stolen, so for Firefox 3.5 and beyond, we reject programs whose
+         * source consists only of XML literals. See:
+         *
+         * https://bugzilla.mozilla.org/show_bug.cgi?id=336551
+         *
+         * The check for this is in jsparse.cpp, JSCompiler::compileScript.
          */
         if ((ts->flags & TSF_OPERAND) &&
             (JS_HAS_XML_OPTION(cx) || PeekChar(ts) != '!')) {
             /* Check for XML comment or CDATA section. */
             if (MatchChar(ts, '!')) {
                 INIT_TOKENBUF();
 
                 /* Scan XML comment. */
--- a/js/src/jsscan.h
+++ b/js/src/jsscan.h
@@ -144,17 +144,21 @@ typedef enum JSTokenType {
     TOK_RESERVED,                       /* reserved keywords */
     TOK_LIMIT                           /* domain size */
 } JSTokenType;
 
 #define IS_PRIMARY_TOKEN(tt) \
     ((uintN)((tt) - TOK_NAME) <= (uintN)(TOK_PRIMARY - TOK_NAME))
 
 #define TOKEN_TYPE_IS_XML(tt) \
-    (tt == TOK_AT || tt == TOK_DBLCOLON || tt == TOK_ANYNAME)
+    ((tt) == TOK_AT || (tt) == TOK_DBLCOLON || (tt) == TOK_ANYNAME)
+
+#define TREE_TYPE_IS_XML(tt)                                                  \
+    ((tt) == TOK_XMLCOMMENT || (tt) == TOK_XMLCDATA || (tt) == TOK_XMLPI ||   \
+     (tt) == TOK_XMLELEM || (tt) == TOK_XMLLIST)
 
 #if JS_HAS_BLOCK_SCOPE
 # define TOKEN_TYPE_IS_DECL(tt) ((tt) == TOK_VAR || (tt) == TOK_LET)
 #else
 # define TOKEN_TYPE_IS_DECL(tt) ((tt) == TOK_VAR)
 #endif
 
 struct JSStringBuffer {