Bug 1418894 - Harden XDR data decoding. r=nbp a=gchang
authorTed Campbell <tcampbell@mozilla.com>
Tue, 28 Nov 2017 23:01:49 -0500
changeset 445127 84ca5dce1d6a63bc64612876eccee30fc45080f9
parent 445126 503237c7de504e66df56ec3c16e2d8e262e11122
child 445128 73bde255ce55e61a3d01873d846d22b50e1a3428
push id1618
push userCallek@gmail.com
push dateThu, 11 Jan 2018 17:45:48 +0000
treeherdermozilla-release@882ca853e05a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnbp, gchang
bugs1418894
milestone58.0
Bug 1418894 - Harden XDR data decoding. r=nbp a=gchang This patch adds better error detection to XDR decoding to reduce memory corruption in the event that XDR data is corrupt (which is not *supposed* to happen). Add missing default cases. Make out-of-range values fail the decode by asserting in debug, and returning a TranscodeError in release. Mix a magic value into enum value before transcoding to buffer (to reduce chance of garbage data being decoded). MozReview-Commit-ID: 1wPkho9dm8c
js/src/jsapi.h
js/src/jsscript.cpp
js/src/shell/js.cpp
js/src/vm/Xdr.h
--- a/js/src/jsapi.h
+++ b/js/src/jsapi.h
@@ -6415,17 +6415,17 @@ enum TranscodeResult
     // Successful encoding / decoding.
     TranscodeResult_Ok = 0,
 
     // A warning message, is set to the message out-param.
     TranscodeResult_Failure = 0x100,
     TranscodeResult_Failure_BadBuildId =          TranscodeResult_Failure | 0x1,
     TranscodeResult_Failure_RunOnceNotSupported = TranscodeResult_Failure | 0x2,
     TranscodeResult_Failure_AsmJSNotSupported =   TranscodeResult_Failure | 0x3,
-    TranscodeResult_Failure_UnknownClassKind =    TranscodeResult_Failure | 0x4,
+    TranscodeResult_Failure_BadDecode =           TranscodeResult_Failure | 0x4,
     TranscodeResult_Failure_WrongCompileOption =  TranscodeResult_Failure | 0x5,
     TranscodeResult_Failure_NotInterpretedFun =   TranscodeResult_Failure | 0x6,
 
     // There is a pending exception on the context.
     TranscodeResult_Throw = 0x200
 };
 
 extern JS_PUBLIC_API(TranscodeResult)
--- a/js/src/jsscript.cpp
+++ b/js/src/jsscript.cpp
@@ -87,34 +87,29 @@ CheckScriptDataIntegrity(JSScript* scrip
 }
 
 template<XDRMode mode>
 bool
 js::XDRScriptConst(XDRState<mode>* xdr, MutableHandleValue vp)
 {
     JSContext* cx = xdr->cx();
 
-    /*
-     * A script constant can be an arbitrary primitive value as they are used
-     * to implement JSOP_LOOKUPSWITCH. But they cannot be objects, see
-     * bug 407186.
-     */
     enum ConstTag {
-        SCRIPT_INT     = 0,
-        SCRIPT_DOUBLE  = 1,
-        SCRIPT_ATOM    = 2,
-        SCRIPT_TRUE    = 3,
-        SCRIPT_FALSE   = 4,
-        SCRIPT_NULL    = 5,
-        SCRIPT_OBJECT  = 6,
-        SCRIPT_VOID    = 7,
-        SCRIPT_HOLE    = 8
+        SCRIPT_INT,
+        SCRIPT_DOUBLE,
+        SCRIPT_ATOM,
+        SCRIPT_TRUE,
+        SCRIPT_FALSE,
+        SCRIPT_NULL,
+        SCRIPT_OBJECT,
+        SCRIPT_VOID,
+        SCRIPT_HOLE
     };
 
-    uint32_t tag;
+    ConstTag tag;
     if (mode == XDR_ENCODE) {
         if (vp.isInt32()) {
             tag = SCRIPT_INT;
         } else if (vp.isDouble()) {
             tag = SCRIPT_DOUBLE;
         } else if (vp.isString()) {
             tag = SCRIPT_ATOM;
         } else if (vp.isTrue()) {
@@ -128,17 +123,17 @@ js::XDRScriptConst(XDRState<mode>* xdr, 
         } else if (vp.isMagic(JS_ELEMENTS_HOLE)) {
             tag = SCRIPT_HOLE;
         } else {
             MOZ_ASSERT(vp.isUndefined());
             tag = SCRIPT_VOID;
         }
     }
 
-    if (!xdr->codeUint32(&tag))
+    if (!xdr->codeEnum32(&tag))
         return false;
 
     switch (tag) {
       case SCRIPT_INT: {
         uint32_t i;
         if (mode == XDR_ENCODE)
             i = uint32_t(vp.toInt32());
         if (!xdr->codeUint32(&i))
@@ -194,16 +189,20 @@ js::XDRScriptConst(XDRState<mode>* xdr, 
       case SCRIPT_VOID:
         if (mode == XDR_DECODE)
             vp.set(UndefinedValue());
         break;
       case SCRIPT_HOLE:
         if (mode == XDR_DECODE)
             vp.setMagic(JS_ELEMENTS_HOLE);
         break;
+      default:
+        // Fail in debug, but only soft-fail in release
+        MOZ_ASSERT(false, "Bad XDR value kind");
+        return xdr->fail(JS::TranscodeResult_Failure_BadDecode);
     }
     return true;
 }
 
 template bool
 js::XDRScriptConst(XDRState<XDR_ENCODE>*, MutableHandleValue);
 
 template bool
@@ -795,16 +794,20 @@ js::XDRScript(XDRState<mode>* xdr, Handl
                 break;
               case ScopeKind::Module:
               case ScopeKind::WasmInstance:
                 MOZ_CRASH("NYI");
                 break;
               case ScopeKind::WasmFunction:
                 MOZ_CRASH("wasm functions cannot be nested in JSScripts");
                 break;
+              default:
+                // Fail in debug, but only soft-fail in release
+                MOZ_ASSERT(false, "Bad XDR scope kind");
+                return xdr->fail(JS::TranscodeResult_Failure_BadDecode);
             }
 
             if (mode == XDR_DECODE)
                 vector[i].init(scope);
         }
     }
 
     /*
@@ -885,18 +888,19 @@ js::XDRScript(XDRState<mode>* xdr, Handl
             RootedObject tmp(cx, *objp);
             if (!XDRObjectLiteral(xdr, &tmp))
                 return false;
             *objp = tmp;
             break;
           }
 
           default: {
-            MOZ_ASSERT(false, "Unknown class kind.");
-            return xdr->fail(JS::TranscodeResult_Failure_UnknownClassKind);
+            // Fail in debug, but only soft-fail in release
+            MOZ_ASSERT(false, "Bad XDR class kind");
+            return xdr->fail(JS::TranscodeResult_Failure_BadDecode);
           }
         }
     }
 
     if (ntrynotes != 0) {
         JSTryNote* tnfirst = script->trynotes()->vector;
         MOZ_ASSERT(script->trynotes()->length == ntrynotes);
         JSTryNote* tn = tnfirst + ntrynotes;
--- a/js/src/shell/js.cpp
+++ b/js/src/shell/js.cpp
@@ -1514,19 +1514,19 @@ ConvertTranscodeResultToJSException(JSCo
       case JS::TranscodeResult_Failure_RunOnceNotSupported:
         MOZ_ASSERT(!cx->isExceptionPending());
         JS_ReportErrorASCII(cx, "run-once script are not supported by XDR");
         return false;
       case JS::TranscodeResult_Failure_AsmJSNotSupported:
         MOZ_ASSERT(!cx->isExceptionPending());
         JS_ReportErrorASCII(cx, "Asm.js is not supported by XDR");
         return false;
-      case JS::TranscodeResult_Failure_UnknownClassKind:
+      case JS::TranscodeResult_Failure_BadDecode:
         MOZ_ASSERT(!cx->isExceptionPending());
-        JS_ReportErrorASCII(cx, "Unknown class kind, go fix it.");
+        JS_ReportErrorASCII(cx, "XDR data corruption");
         return false;
       case JS::TranscodeResult_Failure_WrongCompileOption:
         MOZ_ASSERT(!cx->isExceptionPending());
         JS_ReportErrorASCII(cx, "Compile options differs from Compile options of the encoding");
         return false;
       case JS::TranscodeResult_Failure_NotInterpretedFun:
         MOZ_ASSERT(!cx->isExceptionPending());
         JS_ReportErrorASCII(cx, "Only interepreted functions are supported by XDR");
--- a/js/src/vm/Xdr.h
+++ b/js/src/vm/Xdr.h
@@ -274,23 +274,27 @@ class XDRState : public XDRCoderBase
     /*
      * Use SFINAE to refuse any specialization which is not an enum.  Uses of
      * this function do not have to specialize the type of the enumerated field
      * as C++ will extract the parameterized from the argument list.
      */
     template <typename T>
     bool codeEnum32(T* val, typename mozilla::EnableIf<mozilla::IsEnum<T>::value, T>::Type * = NULL)
     {
+        // Mix the enumeration value with a random magic number, such that a
+        // corruption with a low-ranged value (like 0) is less likely to cause a
+        // miss-interpretation of the XDR content and instead cause a failure.
+        const uint32_t MAGIC = 0x21AB218C;
         uint32_t tmp;
         if (mode == XDR_ENCODE)
-            tmp = uint32_t(*val);
+            tmp = uint32_t(*val) ^ MAGIC;
         if (!codeUint32(&tmp))
             return false;
         if (mode == XDR_DECODE)
-            *val = T(tmp);
+            *val = T(tmp ^ MAGIC);
         return true;
     }
 
     bool codeDouble(double* dp) {
         union DoublePun {
             double d;
             uint64_t u;
         } pun;