From 91d9f5d658d646e1ad1c0b3a28a9bba0094a44c6 Mon Sep 17 00:00:00 2001 From: wolfbeast Date: Fri, 26 Jan 2018 21:26:07 +0100 Subject: Make XDR decoding more robust. --- js/src/jsapi.h | 2 +- js/src/jsfun.cpp | 4 ++++ js/src/jsscript.cpp | 50 ++++++++++++++++++++++++++++++++------------------ js/src/shell/js.cpp | 4 ++-- js/src/vm/Xdr.h | 20 ++++++++++++++++++-- 5 files changed, 57 insertions(+), 23 deletions(-) (limited to 'js') diff --git a/js/src/jsapi.h b/js/src/jsapi.h index 9ad3e757f..089e22b09 100644 --- a/js/src/jsapi.h +++ b/js/src/jsapi.h @@ -5927,7 +5927,7 @@ enum TranscodeResult 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, // A error, the JSContext has a pending exception. TranscodeResult_Throw = 0x200 diff --git a/js/src/jsfun.cpp b/js/src/jsfun.cpp index 9fffce448..c952441ad 100644 --- a/js/src/jsfun.cpp +++ b/js/src/jsfun.cpp @@ -641,6 +641,10 @@ js::XDRInterpretedFunction(XDRState* xdr, HandleScope enclosingScope, objp.set(fun); } + // Verify marker at end of function to detect buffer truncation. + if (!xdr->codeMarker(0xA0129CD1)) + return false; + return true; } diff --git a/js/src/jsscript.cpp b/js/src/jsscript.cpp index 2e02aa63d..10821f26a 100644 --- a/js/src/jsscript.cpp +++ b/js/src/jsscript.cpp @@ -75,24 +75,19 @@ js::XDRScriptConst(XDRState* 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; @@ -116,7 +111,7 @@ js::XDRScriptConst(XDRState* xdr, MutableHandleValue vp) } } - if (!xdr->codeUint32(&tag)) + if (!xdr->codeEnum32(&tag)) return false; switch (tag) { @@ -182,6 +177,10 @@ js::XDRScriptConst(XDRState* xdr, MutableHandleValue vp) 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; } @@ -742,11 +741,20 @@ js::XDRScript(XDRState* xdr, HandleScope scriptEnclosingScope, HandleScrip case ScopeKind::Module: MOZ_CRASH("NYI"); 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); } + + // Verify marker to detect data corruption after decoding scope data. A + // mismatch here indicates we will almost certainly crash in release. + if (!xdr->codeMarker(0xF81F7F5A)) + return false; } /* @@ -832,12 +840,18 @@ js::XDRScript(XDRState* xdr, HandleScope scriptEnclosingScope, HandleScrip } 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); } } } + // Verify marker to detect data corruption after decoding object data. A + // mismatch here indicates we will almost certainly crash in release. + if (!xdr->codeMarker(0x223DB179)) + return false; + if (ntrynotes != 0) { JSTryNote* tnfirst = script->trynotes()->vector; MOZ_ASSERT(script->trynotes()->length == ntrynotes); diff --git a/js/src/shell/js.cpp b/js/src/shell/js.cpp index 88d482a23..3f56981cd 100644 --- a/js/src/shell/js.cpp +++ b/js/src/shell/js.cpp @@ -1546,9 +1546,9 @@ ConvertTranscodeResultToJSException(JSContext* cx, JS::TranscodeResult rv) 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_Throw: diff --git a/js/src/vm/Xdr.h b/js/src/vm/Xdr.h index 8e8c5bf17..2a5c62480 100644 --- a/js/src/vm/Xdr.h +++ b/js/src/vm/Xdr.h @@ -143,13 +143,17 @@ class XDRState { template bool codeEnum32(T* val, typename mozilla::EnableIf::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 = 0xAF647BCE; 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; } @@ -167,6 +171,18 @@ class XDRState { return true; } + bool codeMarker(uint32_t magic) { + uint32_t actual = magic; + if (!codeUint32(&actual)) + return false; + if (actual != magic) { + // Fail in debug, but only soft-fail in release + MOZ_ASSERT(false, "Bad XDR marker"); + return fail(JS::TranscodeResult_Failure_BadDecode); + } + return true; + } + bool codeBytes(void* bytes, size_t len) { if (len == 0) return true; -- cgit v1.2.3