From 770748abfb8a86f5e1c70ae1c2d17ff1e86b5871 Mon Sep 17 00:00:00 2001 From: Nick Fitzgerald Date: Fri, 13 Feb 2015 09:21:50 -0800 Subject: [PATCH] Bug 1117242 - SavedFrame objects should do principal checks for every accessor; r=jimb,jandem,bz --- dom/base/DOMException.cpp | 9 +- dom/bindings/Exceptions.cpp | 63 ++++- js/src/builtin/TestingFunctions.cpp | 35 ++- js/src/doc/SavedFrame/SavedFrame.md | 38 +++ js/src/gc/Rooting.h | 2 + .../tests/saved-stacks/principals-01.js | 2 +- .../tests/saved-stacks/principals-02.js | 2 +- .../tests/saved-stacks/principals-03.js | 23 ++ .../tests/saved-stacks/principals-04.js | 15 + js/src/jsfriendapi.h | 11 + js/src/jsprototypes.h | 2 + js/src/jsscript.cpp | 2 - js/src/vm/GlobalObject.h | 15 +- js/src/vm/SavedStacks.cpp | 261 ++++++++++-------- js/src/vm/SavedStacks.h | 27 +- js/xpconnect/src/xpcprivate.h | 2 +- .../tests/unit/test_xray_SavedFrame.js | 108 ++++++++ js/xpconnect/tests/unit/xpcshell.ini | 1 + js/xpconnect/wrappers/XrayWrapper.cpp | 1 + xpcom/base/nsIException.idl | 6 +- 20 files changed, 469 insertions(+), 156 deletions(-) create mode 100644 js/src/jit-test/tests/saved-stacks/principals-03.js create mode 100644 js/src/jit-test/tests/saved-stacks/principals-04.js create mode 100644 js/xpconnect/tests/unit/test_xray_SavedFrame.js diff --git a/dom/base/DOMException.cpp b/dom/base/DOMException.cpp index 1757199c42f..cedb9028c62 100644 --- a/dom/base/DOMException.cpp +++ b/dom/base/DOMException.cpp @@ -739,11 +739,10 @@ DOMException::Sanitize(JSContext* aCx, // Now it's possible that the stack on retval still starts with // stuff aCx is not supposed to touch; it depends on what's on the // stack right this second. Walk past all of that. - while (retval->mLocation && !retval->mLocation->CallerSubsumes(aCx)) { - nsCOMPtr caller; - retval->mLocation->GetCaller(getter_AddRefs(caller)); - retval->mLocation.swap(caller); - } + nsCOMPtr stack; + nsresult rv = retval->mLocation->GetSanitized(aCx, getter_AddRefs(stack)); + NS_ENSURE_SUCCESS(rv, false); + retval->mLocation.swap(stack); } return ToJSValue(aCx, retval, aSanitizedValue); diff --git a/dom/bindings/Exceptions.cpp b/dom/bindings/Exceptions.cpp index 01fbeaf31bd..f228570d972 100644 --- a/dom/bindings/Exceptions.cpp +++ b/dom/bindings/Exceptions.cpp @@ -6,6 +6,7 @@ #include "mozilla/dom/Exceptions.h" #include "js/GCAPI.h" +#include "js/TypeDecls.h" #include "jsapi.h" #include "jsprf.h" #include "mozilla/CycleCollectedJSRuntime.h" @@ -299,6 +300,8 @@ public: NS_IMETHOD GetCaller(nsIStackFrame** aCaller) MOZ_OVERRIDE; NS_IMETHOD GetFormattedStack(nsAString& aStack) MOZ_OVERRIDE; virtual bool CallerSubsumes(JSContext* aCx) MOZ_OVERRIDE; + NS_IMETHOD GetSanitized(JSContext* aCx, + nsIStackFrame** aSanitized) MOZ_OVERRIDE; protected: virtual bool IsJSFrame() const MOZ_OVERRIDE { @@ -390,15 +393,22 @@ NS_IMETHODIMP JSStackFrame::GetFilename(nsAString& aFilename) JS::Rooted stack(cx, mStack); JS::ExposeObjectToActiveJS(mStack); JSAutoCompartment ac(cx, stack); + JS::Rooted filenameVal(cx); - if (!JS_GetProperty(cx, stack, "source", &filenameVal) || - !filenameVal.isString()) { + if (!JS_GetProperty(cx, stack, "source", &filenameVal)) { return NS_ERROR_UNEXPECTED; } + + if (filenameVal.isNull()) { + filenameVal = JS_GetEmptyStringValue(cx); + } + MOZ_ASSERT(filenameVal.isString()); + nsAutoJSString str; if (!str.init(cx, filenameVal.toString())) { return NS_ERROR_OUT_OF_MEMORY; } + mFilename = str; mFilenameInitialized = true; } @@ -471,11 +481,15 @@ JSStackFrame::GetLineno(int32_t* aLineNo) JS::ExposeObjectToActiveJS(mStack); JSAutoCompartment ac(cx, stack); JS::Rooted lineVal(cx); - if (!JS_GetProperty(cx, stack, "line", &lineVal) || - !lineVal.isNumber()) { + if (!JS_GetProperty(cx, stack, "line", &lineVal)) { return NS_ERROR_UNEXPECTED; } - mLineno = lineVal.toNumber(); + if (lineVal.isNumber()) { + mLineno = lineVal.toNumber(); + } else { + MOZ_ASSERT(lineVal.isNull()); + mLineno = 0; + } mLinenoInitialized = true; } @@ -500,11 +514,15 @@ JSStackFrame::GetColNo(int32_t* aColNo) JS::ExposeObjectToActiveJS(mStack); JSAutoCompartment ac(cx, stack); JS::Rooted colVal(cx); - if (!JS_GetProperty(cx, stack, "column", &colVal) || - !colVal.isNumber()) { + if (!JS_GetProperty(cx, stack, "column", &colVal)) { return NS_ERROR_UNEXPECTED; } - mColNo = colVal.toNumber(); + if (colVal.isNumber()) { + mColNo = colVal.toNumber(); + } else { + MOZ_ASSERT(colVal.isNull()); + mColNo = 0; + } mColNoInitialized = true; } @@ -524,6 +542,35 @@ NS_IMETHODIMP StackFrame::GetSourceLine(nsACString& aSourceLine) return NS_OK; } +/* [noscript] readonly attribute nsIStackFrame sanitized */ +NS_IMETHODIMP StackFrame::GetSanitized(JSContext*, nsIStackFrame** aSanitized) +{ + NS_ADDREF(*aSanitized = this); + return NS_OK; +} + +/* [noscript] readonly attribute nsIStackFrame sanitized */ +NS_IMETHODIMP JSStackFrame::GetSanitized(JSContext* aCx, nsIStackFrame** aSanitized) +{ + // NB: Do _not_ enter the compartment of the SavedFrame object here, because + // we are checking against the caller's compartment's principals in + // GetFirstSubsumedSavedFrame. + + JS::RootedObject savedFrame(aCx, mStack); + JS::ExposeObjectToActiveJS(mStack); + + savedFrame = js::GetFirstSubsumedSavedFrame(aCx, savedFrame); + nsCOMPtr stackFrame; + if (savedFrame) { + stackFrame = new JSStackFrame(savedFrame); + } else { + stackFrame = new StackFrame(); + } + + stackFrame.forget(aSanitized); + return NS_OK; +} + /* readonly attribute nsIStackFrame caller; */ NS_IMETHODIMP JSStackFrame::GetCaller(nsIStackFrame** aCaller) { diff --git a/js/src/builtin/TestingFunctions.cpp b/js/src/builtin/TestingFunctions.cpp index 2af889443b7..5c7df16e14a 100644 --- a/js/src/builtin/TestingFunctions.cpp +++ b/js/src/builtin/TestingFunctions.cpp @@ -967,9 +967,30 @@ SaveStack(JSContext *cx, unsigned argc, jsval *vp) maxFrameCount = d; } - Rooted stack(cx); - if (!JS::CaptureCurrentStack(cx, &stack, maxFrameCount)) + JSCompartment *targetCompartment = cx->compartment(); + if (args.length() >= 2) { + if (!args[1].isObject()) { + js_ReportValueErrorFlags(cx, JSREPORT_ERROR, JSMSG_UNEXPECTED_TYPE, + JSDVG_SEARCH_STACK, args[0], JS::NullPtr(), + "not an object", NULL); + return false; + } + RootedObject obj(cx, UncheckedUnwrap(&args[1].toObject())); + if (!obj) + return false; + targetCompartment = obj->compartment(); + } + + RootedObject stack(cx); + { + AutoCompartment ac(cx, targetCompartment); + if (!JS::CaptureCurrentStack(cx, &stack, maxFrameCount)) + return false; + } + + if (stack && !cx->compartment()->wrap(cx, &stack)) return false; + args.rval().setObjectOrNull(stack); return true; } @@ -2396,13 +2417,15 @@ static const JSFunctionSpecWithHelp TestingFunctions[] = { " SavedStacks cache."), JS_FN_HELP("saveStack", SaveStack, 0, 0, -"saveStack()", -" Capture a stack.\n"), +"saveStack([maxDepth [, compartment]])", +" Capture a stack. If 'maxDepth' is given, capture at most 'maxDepth' number\n" +" of frames. If 'compartment' is given, allocate the js::SavedFrame instances\n" +" with the given object's compartment."), JS_FN_HELP("enableTrackAllocations", EnableTrackAllocations, 0, 0, "enableTrackAllocations()", -" Start capturing the JS stack at every allocation. Note that this sets an " -" object metadata callback that will override any other object metadata " +" Start capturing the JS stack at every allocation. Note that this sets an\n" +" object metadata callback that will override any other object metadata\n" " callback that may be set."), JS_FN_HELP("disableTrackAllocations", DisableTrackAllocations, 0, 0, diff --git a/js/src/doc/SavedFrame/SavedFrame.md b/js/src/doc/SavedFrame/SavedFrame.md index 2bad954f35a..3dcafe998a8 100644 --- a/js/src/doc/SavedFrame/SavedFrame.md +++ b/js/src/doc/SavedFrame/SavedFrame.md @@ -5,6 +5,44 @@ JavaScript call stack at a past moment of execution. Younger frames hold a reference to the frames that invoked them. The older tails are shared across many younger frames. +`SavedFrame` stacks should generally be captured, allocated, and live within the +compartment that is being observed or debugged. Usually this is a content +compartment. + +## Capturing `SavedFrame` Stacks + +### From C++ + +Use `JS::CaptureCurrentStack` declared in `jsapi.h`. + +### From JS + +Use `saveStack`, accessible via `Components.utils.getJSTestingFunction()`. + +## Including and Excluding Chrome Frames + +Consider the following `SavedFrame` stack. Arrows represent links from child to +parent frame, `content.js` is from a compartment with content principals, and +`chrome.js` is from a compartment with chrome principals. + + function A from content.js + | + V + function B from chrome.js + | + V + function C from content.js + +The content compartment will ever have one view of this stack: `A -> C`. + +However, a chrome compartment has a choice: it can either take the same view +that the content compartment has (`A -> C`), or it can view all stack frames, +including the frames from chrome compartments (`A -> B -> C`). To view +everything, use an `XrayWrapper`. This is the default wrapper. To see the stack +as the content compartment sees it, waive the xray wrapper with +`Components.utils.waiveXrays`: + + const contentViewOfStack = Components.utils.waiveXrays(someStack); ## Accessor Properties of the `SavedFrame.prototype` Object diff --git a/js/src/gc/Rooting.h b/js/src/gc/Rooting.h index ab38313521c..4c0d18f10a9 100644 --- a/js/src/gc/Rooting.h +++ b/js/src/gc/Rooting.h @@ -17,6 +17,7 @@ namespace js { class PropertyName; class NativeObject; class ArrayObject; +class GlobalObject; class PlainObject; class ScriptSourceObject; class Shape; @@ -45,6 +46,7 @@ typedef JS::Rooted RootedAtom; typedef JS::Rooted RootedLinearString; typedef JS::Rooted RootedPropertyName; typedef JS::Rooted RootedArrayObject; +typedef JS::Rooted RootedGlobalObject; typedef JS::Rooted RootedPlainObject; typedef JS::Rooted RootedScriptSource; diff --git a/js/src/jit-test/tests/saved-stacks/principals-01.js b/js/src/jit-test/tests/saved-stacks/principals-01.js index 1c67956b2df..62640472dc3 100644 --- a/js/src/jit-test/tests/saved-stacks/principals-01.js +++ b/js/src/jit-test/tests/saved-stacks/principals-01.js @@ -42,7 +42,7 @@ var count = 0; low .eval('function b() { check("b", extract(saveStack())); c(); }'); mid .eval('function c() { check("cba", extract(saveStack())); d(); }'); high.eval('function d() { check("dcba", extract(saveStack())); e(); }'); - eval('function e() { check("edcba", extract(saveStack())); f(); }'); // no principal, so checks skipped + eval('function e() { check("ecba", extract(saveStack())); f(); }'); low .eval('function f() { check("fb", extract(saveStack())); g(); }'); mid .eval('function g() { check("gfecba", extract(saveStack())); h(); }'); high.eval('function h() { check("hgfedcba", extract(saveStack())); }'); diff --git a/js/src/jit-test/tests/saved-stacks/principals-02.js b/js/src/jit-test/tests/saved-stacks/principals-02.js index 796c48b0694..24ffa82a875 100644 --- a/js/src/jit-test/tests/saved-stacks/principals-02.js +++ b/js/src/jit-test/tests/saved-stacks/principals-02.js @@ -33,7 +33,7 @@ var high = newGlobal({ principal: 0xfffff }); low .eval('function b() { check("b", saveStack().toString()); c(); }'); mid .eval('function c() { check("cba", saveStack().toString()); d(); }'); high.eval('function d() { check("dcba", saveStack().toString()); e(); }'); - eval('function e() { check("edcba", saveStack().toString()); f(); }'); // no principal, so checks skipped + eval('function e() { check("ecba", saveStack().toString()); f(); }'); low .eval('function f() { check("fb", saveStack().toString()); g(); }'); mid .eval('function g() { check("gfecba", saveStack().toString()); h(); }'); high.eval('function h() { check("hgfedcba", saveStack().toString()); }'); diff --git a/js/src/jit-test/tests/saved-stacks/principals-03.js b/js/src/jit-test/tests/saved-stacks/principals-03.js new file mode 100644 index 00000000000..006b4477cd6 --- /dev/null +++ b/js/src/jit-test/tests/saved-stacks/principals-03.js @@ -0,0 +1,23 @@ +// With arrows representing child-to-parent links, create a SavedFrame stack +// like this: +// +// high.a -> low.b +// +// in `low`'s compartment and give `low` a reference to this stack. Assert the +// stack's youngest frame's properties doesn't leak information about `high.a` +// that `low` shouldn't have access to, and instead returns information about +// `low.b`. + +var low = newGlobal({ principal: 0 }); +var high = newGlobal({ principal: 0xfffff }); + +low.high = high; +high.low = low; + +high.eval("function a() { return saveStack(0, low); }"); +low.eval("function b() { return high.a(); }") + +var stack = low.b(); + +assertEq(stack.functionDisplayName, "b"); +assertEq(stack.parent, null); diff --git a/js/src/jit-test/tests/saved-stacks/principals-04.js b/js/src/jit-test/tests/saved-stacks/principals-04.js new file mode 100644 index 00000000000..3a9b5780058 --- /dev/null +++ b/js/src/jit-test/tests/saved-stacks/principals-04.js @@ -0,0 +1,15 @@ +// Test what happens when a compartment gets a SavedFrame that it doesn't have +// the principals to access any of its frames. + +var low = newGlobal({ principal: 0 }); +var high = newGlobal({ principal: 0xfffff }); + +low.high = high; +high.low = low; + +high.eval("function a() { return saveStack(1, low); }"); +var stack = low.eval("high.a();") + +assertEq(stack.functionDisplayName, null); +assertEq(stack.parent, null); +assertEq(stack.toString(), ""); diff --git a/js/src/jsfriendapi.h b/js/src/jsfriendapi.h index 096f4242f60..caf18f8ba08 100644 --- a/js/src/jsfriendapi.h +++ b/js/src/jsfriendapi.h @@ -2622,6 +2622,17 @@ GetObjectEnvironmentObjectForFunction(JSFunction *fun); extern JS_FRIEND_API(JSPrincipals *) GetSavedFramePrincipals(JS::HandleObject savedFrame); +/* + * Get the first SavedFrame object in this SavedFrame stack whose principals are + * subsumed by the cx's principals. If there is no such frame, return nullptr. + * + * Do NOT pass a non-SavedFrame object here. + * + * The savedFrame and cx do not need to be in the same compartment. + */ +extern JS_FRIEND_API(JSObject *) +GetFirstSubsumedSavedFrame(JSContext *cx, JS::HandleObject savedFrame); + } /* namespace js */ extern JS_FRIEND_API(bool) diff --git a/js/src/jsprototypes.h b/js/src/jsprototypes.h index 467ee57608c..07adfed4125 100644 --- a/js/src/jsprototypes.h +++ b/js/src/jsprototypes.h @@ -111,6 +111,8 @@ IF_SAB(real,imaginary)(SharedFloat64Array, 50, js_InitViaClassSpec, IF_SAB(real,imaginary)(SharedUint8ClampedArray, 51, js_InitViaClassSpec, SHARED_TYPED_ARRAY_CLASP(Uint8Clamped)) \ real(TypedArray, 52, js_InitViaClassSpec, &js::TypedArrayObject::sharedTypedArrayPrototypeClass) \ IF_SAB(real,imaginary)(Atomics, 53, js_InitAtomicsClass, OCLASP(Atomics)) \ + real(SavedFrame, 54, js_InitViaClassSpec, &js::SavedFrame::class_) \ + #define JS_FOR_EACH_PROTOTYPE(macro) JS_FOR_PROTOTYPES(macro,macro) diff --git a/js/src/jsscript.cpp b/js/src/jsscript.cpp index 0af4882c518..edcc351015c 100644 --- a/js/src/jsscript.cpp +++ b/js/src/jsscript.cpp @@ -60,8 +60,6 @@ using mozilla::PodCopy; using mozilla::PodZero; using mozilla::RotateLeft; -typedef Rooted RootedGlobalObject; - /* static */ BindingIter Bindings::argumentsBinding(ExclusiveContext *cx, InternalBindingsHandle bindings) { diff --git a/js/src/vm/GlobalObject.h b/js/src/vm/GlobalObject.h index 3e15a813cdf..9b0332222c7 100644 --- a/js/src/vm/GlobalObject.h +++ b/js/src/vm/GlobalObject.h @@ -321,7 +321,7 @@ class GlobalObject : public NativeObject NativeObject *getOrCreateObjectPrototype(JSContext *cx) { if (functionObjectClassesInitialized()) return &getPrototype(JSProto_Object).toObject().as(); - Rooted self(cx, this); + RootedGlobalObject self(cx, this); if (!ensureConstructor(cx, self, JSProto_Object)) return nullptr; return &self->getPrototype(JSProto_Object).toObject().as(); @@ -330,7 +330,7 @@ class GlobalObject : public NativeObject NativeObject *getOrCreateFunctionPrototype(JSContext *cx) { if (functionObjectClassesInitialized()) return &getPrototype(JSProto_Function).toObject().as(); - Rooted self(cx, this); + RootedGlobalObject self(cx, this); if (!ensureConstructor(cx, self, JSProto_Object)) return nullptr; return &self->getPrototype(JSProto_Function).toObject().as(); @@ -384,6 +384,13 @@ class GlobalObject : public NativeObject return nullptr; } + static NativeObject *getOrCreateSavedFramePrototype(JSContext *cx, + Handle global) { + if (!ensureConstructor(cx, global, JSProto_SavedFrame)) + return nullptr; + return &global->getPrototype(JSProto_SavedFrame).toObject().as(); + } + static JSObject *getOrCreateArrayBufferPrototype(JSContext *cx, Handle global) { if (!ensureConstructor(cx, global, JSProto_ArrayBuffer)) return nullptr; @@ -483,7 +490,7 @@ class GlobalObject : public NativeObject Value v = getSlotRef(slot); if (v.isObject()) return &v.toObject(); - Rooted self(cx, this); + RootedGlobalObject self(cx, this); if (!init(cx, self)) return nullptr; return &self->getSlot(slot).toObject(); @@ -561,7 +568,7 @@ class GlobalObject : public NativeObject } JSObject *getOrCreateDataViewPrototype(JSContext *cx) { - Rooted self(cx, this); + RootedGlobalObject self(cx, this); if (!ensureConstructor(cx, self, JSProto_DataView)) return nullptr; return &self->getPrototype(JSProto_DataView).toObject(); diff --git a/js/src/vm/SavedStacks.cpp b/js/src/vm/SavedStacks.cpp index 9bc5eac1d77..b6c34a6ef06 100644 --- a/js/src/vm/SavedStacks.cpp +++ b/js/src/vm/SavedStacks.cpp @@ -16,12 +16,13 @@ #include "jshashutil.h" #include "jsmath.h" #include "jsnum.h" +#include "jsscript.h" #include "prmjtime.h" #include "gc/Marking.h" +#include "gc/Rooting.h" #include "js/Vector.h" #include "vm/Debugger.h" -#include "vm/GlobalObject.h" #include "vm/StringBuffer.h" #include "jscntxtinlines.h" @@ -136,18 +137,67 @@ SavedFrame::HashPolicy::rekey(Key &key, const Key &newKey) key = newKey; } +/* static */ bool +SavedFrame::finishSavedFrameInit(JSContext *cx, HandleObject ctor, HandleObject proto) +{ + // The only object with the SavedFrame::class_ that doesn't have a source + // should be the prototype. + proto->as().setReservedSlot(SavedFrame::JSSLOT_SOURCE, NullValue()); + + return FreezeObject(cx, proto); +} + /* static */ const Class SavedFrame::class_ = { "SavedFrame", JSCLASS_HAS_PRIVATE | JSCLASS_IMPLEMENTS_BARRIERS | - JSCLASS_HAS_RESERVED_SLOTS(SavedFrame::JSSLOT_COUNT), - nullptr, // addProperty - nullptr, // delProperty - nullptr, // getProperty - nullptr, // setProperty - nullptr, // enumerate - nullptr, // resolve - nullptr, // convert - SavedFrame::finalize + JSCLASS_HAS_RESERVED_SLOTS(SavedFrame::JSSLOT_COUNT) | + JSCLASS_HAS_CACHED_PROTO(JSProto_SavedFrame) | + JSCLASS_IS_ANONYMOUS, + nullptr, // addProperty + nullptr, // delProperty + nullptr, // getProperty + nullptr, // setProperty + nullptr, // enumerate + nullptr, // resolve + nullptr, // convert + SavedFrame::finalize, // finalize + nullptr, // call + nullptr, // hasInstance + nullptr, // construct + nullptr, // trace + + // ClassSpec + { + GenericCreateConstructor, + GenericCreatePrototype, + SavedFrame::staticFunctions, + SavedFrame::protoFunctions, + SavedFrame::protoAccessors, + SavedFrame::finishSavedFrameInit, + ClassSpec::DontDefineConstructor + } +}; + +/* static */ const JSFunctionSpec +SavedFrame::staticFunctions[] = { + JS_FS_END +}; + +/* static */ const JSFunctionSpec +SavedFrame::protoFunctions[] = { + JS_FN("constructor", SavedFrame::construct, 0, 0), + JS_FN("toString", SavedFrame::toStringMethod, 0, 0), + JS_FS_END +}; + +/* static */ const JSPropertySpec +SavedFrame::protoAccessors[] = { + JS_PSG("source", SavedFrame::sourceProperty, 0), + JS_PSG("line", SavedFrame::lineProperty, 0), + JS_PSG("column", SavedFrame::columnProperty, 0), + JS_PSG("functionDisplayName", SavedFrame::functionDisplayNameProperty, 0), + JS_PSG("parent", SavedFrame::parentProperty, 0), + JS_PS_END }; /* static */ void @@ -259,33 +309,67 @@ SavedFrame::construct(JSContext *cx, unsigned argc, Value *vp) return false; } -/* static */ SavedFrame * -SavedFrame::checkThis(JSContext *cx, CallArgs &args, const char *fnName) +// Return the first SavedFrame in the chain that starts with |frame| whose +// principals are subsumed by |principals|, according to |subsumes|. If there is +// no such frame, return nullptr. +static SavedFrame * +GetFirstSubsumedFrame(JSContext *cx, HandleSavedFrame frame) +{ + JSSubsumesOp subsumes = cx->runtime()->securityCallbacks->subsumes; + if (!subsumes) + return frame; + + JSPrincipals *principals = cx->compartment()->principals; + + RootedSavedFrame rootedFrame(cx, frame); + while (rootedFrame && !subsumes(principals, rootedFrame->getPrincipals())) + rootedFrame = rootedFrame->getParent(); + + return rootedFrame; +} + +JS_FRIEND_API(JSObject *) +GetFirstSubsumedSavedFrame(JSContext *cx, HandleObject savedFrame) +{ + if (!savedFrame) + return nullptr; + RootedSavedFrame frame(cx, &savedFrame->as()); + return GetFirstSubsumedFrame(cx, frame); +} + +/* static */ bool +SavedFrame::checkThis(JSContext *cx, CallArgs &args, const char *fnName, + MutableHandleSavedFrame frame) { const Value &thisValue = args.thisv(); if (!thisValue.isObject()) { JS_ReportErrorNumber(cx, js_GetErrorMessage, nullptr, JSMSG_NOT_NONNULL_OBJECT); - return nullptr; + return false; } - JSObject &thisObject = thisValue.toObject(); - if (!thisObject.is()) { + JSObject *thisObject = CheckedUnwrap(&thisValue.toObject()); + if (!thisObject || !thisObject->is()) { JS_ReportErrorNumber(cx, js_GetErrorMessage, nullptr, JSMSG_INCOMPATIBLE_PROTO, - SavedFrame::class_.name, fnName, thisObject.getClass()->name); - return nullptr; + SavedFrame::class_.name, fnName, + thisObject ? thisObject->getClass()->name : "object"); + return false; } // Check for SavedFrame.prototype, which has the same class as SavedFrame // instances, however doesn't actually represent a captured stack frame. It // is the only object that is() but doesn't have a source. - if (thisObject.as().getReservedSlot(JSSLOT_SOURCE).isNull()) { + if (thisObject->as().getReservedSlot(JSSLOT_SOURCE).isNull()) { JS_ReportErrorNumber(cx, js_GetErrorMessage, nullptr, JSMSG_INCOMPATIBLE_PROTO, SavedFrame::class_.name, fnName, "prototype object"); - return nullptr; + return false; } - return &thisObject.as(); + // The caller might not have the principals to see this frame's data, so get + // the first one they _do_ have access to. + RootedSavedFrame rooted(cx, &thisObject->as()); + frame.set(GetFirstSubsumedFrame(cx, rooted)); + return true; } // Get the SavedFrame * from the current this value and handle any errors that @@ -296,19 +380,24 @@ SavedFrame::checkThis(JSContext *cx, CallArgs &args, const char *fnName) // - unsigned argc // - Value *vp // - const char *fnName +// - Value defaultVal // These parameters will be defined after calling this macro: // - CallArgs args // - Rooted frame (will be non-null) -#define THIS_SAVEDFRAME(cx, argc, vp, fnName, args, frame) \ - CallArgs args = CallArgsFromVp(argc, vp); \ - RootedSavedFrame frame(cx, checkThis(cx, args, fnName)); \ - if (!frame) \ - return false +#define THIS_SAVEDFRAME(cx, argc, vp, fnName, defaultVal, args, frame) \ + CallArgs args = CallArgsFromVp(argc, vp); \ + RootedSavedFrame frame(cx); \ + if (!checkThis(cx, args, fnName, &frame)) \ + return false; \ + if (!frame) { \ + args.rval().set(defaultVal); \ + return true; \ + } /* static */ bool SavedFrame::sourceProperty(JSContext *cx, unsigned argc, Value *vp) { - THIS_SAVEDFRAME(cx, argc, vp, "(get source)", args, frame); + THIS_SAVEDFRAME(cx, argc, vp, "(get source)", NullValue(), args, frame); args.rval().setString(frame->getSource()); return true; } @@ -316,7 +405,7 @@ SavedFrame::sourceProperty(JSContext *cx, unsigned argc, Value *vp) /* static */ bool SavedFrame::lineProperty(JSContext *cx, unsigned argc, Value *vp) { - THIS_SAVEDFRAME(cx, argc, vp, "(get line)", args, frame); + THIS_SAVEDFRAME(cx, argc, vp, "(get line)", NullValue(), args, frame); uint32_t line = frame->getLine(); args.rval().setNumber(line); return true; @@ -325,7 +414,7 @@ SavedFrame::lineProperty(JSContext *cx, unsigned argc, Value *vp) /* static */ bool SavedFrame::columnProperty(JSContext *cx, unsigned argc, Value *vp) { - THIS_SAVEDFRAME(cx, argc, vp, "(get column)", args, frame); + THIS_SAVEDFRAME(cx, argc, vp, "(get column)", NullValue(), args, frame); uint32_t column = frame->getColumn(); args.rval().setNumber(column); return true; @@ -334,7 +423,7 @@ SavedFrame::columnProperty(JSContext *cx, unsigned argc, Value *vp) /* static */ bool SavedFrame::functionDisplayNameProperty(JSContext *cx, unsigned argc, Value *vp) { - THIS_SAVEDFRAME(cx, argc, vp, "(get functionDisplayName)", args, frame); + THIS_SAVEDFRAME(cx, argc, vp, "(get functionDisplayName)", NullValue(), args, frame); RootedAtom name(cx, frame->getFunctionDisplayName()); if (name) args.rval().setString(name); @@ -346,55 +435,45 @@ SavedFrame::functionDisplayNameProperty(JSContext *cx, unsigned argc, Value *vp) /* static */ bool SavedFrame::parentProperty(JSContext *cx, unsigned argc, Value *vp) { - THIS_SAVEDFRAME(cx, argc, vp, "(get parent)", args, frame); - JSSubsumesOp subsumes = cx->runtime()->securityCallbacks->subsumes; - JSPrincipals *principals = cx->compartment()->principals; - - do - frame = frame->getParent(); - while (frame && principals && subsumes && - !subsumes(principals, frame->getPrincipals())); - - args.rval().setObjectOrNull(frame); + THIS_SAVEDFRAME(cx, argc, vp, "(get parent)", NullValue(), args, frame); + RootedSavedFrame parent(cx, frame->getParent()); + args.rval().setObjectOrNull(GetFirstSubsumedFrame(cx, parent)); return true; } -/* static */ const JSPropertySpec SavedFrame::properties[] = { - JS_PSG("source", SavedFrame::sourceProperty, 0), - JS_PSG("line", SavedFrame::lineProperty, 0), - JS_PSG("column", SavedFrame::columnProperty, 0), - JS_PSG("functionDisplayName", SavedFrame::functionDisplayNameProperty, 0), - JS_PSG("parent", SavedFrame::parentProperty, 0), - JS_PS_END -}; - /* static */ bool SavedFrame::toStringMethod(JSContext *cx, unsigned argc, Value *vp) { - THIS_SAVEDFRAME(cx, argc, vp, "toString", args, frame); + THIS_SAVEDFRAME(cx, argc, vp, "toString", StringValue(cx->runtime()->emptyString), args, frame); StringBuffer sb(cx); - JSSubsumesOp subsumes = cx->runtime()->securityCallbacks->subsumes; - JSPrincipals *principals = cx->compartment()->principals; + DebugOnly subsumes = cx->runtime()->securityCallbacks->subsumes; + DebugOnly principals = cx->compartment()->principals; + RootedSavedFrame parent(cx); do { - if (principals && subsumes && !subsumes(principals, frame->getPrincipals())) - continue; + MOZ_ASSERT_IF(subsumes, (*subsumes)(principals, frame->getPrincipals())); if (frame->isSelfHosted()) - continue; + goto nextIteration; - RootedAtom name(cx, frame->getFunctionDisplayName()); - if ((name && !sb.append(name)) - || !sb.append('@') - || !sb.append(frame->getSource()) - || !sb.append(':') - || !NumberValueToStringBuffer(cx, NumberValue(frame->getLine()), sb) - || !sb.append(':') - || !NumberValueToStringBuffer(cx, NumberValue(frame->getColumn()), sb) - || !sb.append('\n')) { - return false; + RootedAtom name(cx, frame->getFunctionDisplayName()); + if ((name && !sb.append(name)) + || !sb.append('@') + || !sb.append(frame->getSource()) + || !sb.append(':') + || !NumberValueToStringBuffer(cx, NumberValue(frame->getLine()), sb) + || !sb.append(':') + || !NumberValueToStringBuffer(cx, NumberValue(frame->getColumn()), sb) + || !sb.append('\n')) + { + return false; + } } - } while ((frame = frame->getParent())); + + nextIteration: + parent = frame->getParent(); + frame = GetFirstSubsumedFrame(cx, parent); + } while (frame); JSString *str = sb.finishString(); if (!str) @@ -403,12 +482,6 @@ SavedFrame::toStringMethod(JSContext *cx, unsigned argc, Value *vp) return true; } -/* static */ const JSFunctionSpec SavedFrame::methods[] = { - JS_FN("constructor", SavedFrame::construct, 0, 0), - JS_FN("toString", SavedFrame::toStringMethod, 0, 0), - JS_FS_END -}; - bool SavedStacks::init() { @@ -460,12 +533,6 @@ SavedStacks::sweep(JSRuntime *rt) } sweepPCLocationMap(); - - if (savedFrameProto.unbarrieredGet() && - IsObjectAboutToBeFinalizedFromAnyThread(savedFrameProto.unsafeGet())) - { - savedFrameProto.set(nullptr); - } } void @@ -587,49 +654,17 @@ SavedStacks::getOrCreateSavedFrame(JSContext *cx, SavedFrame::HandleLookup looku return frame; } -JSObject * -SavedStacks::getOrCreateSavedFramePrototype(JSContext *cx) -{ - if (savedFrameProto) - return savedFrameProto; - - Rooted global(cx, cx->compartment()->maybeGlobal()); - if (!global) - return nullptr; - - Rooted proto(cx, - NewObjectWithGivenProto(cx, global->getOrCreateObjectPrototype(cx), global)); - if (!proto - || !JS_DefineProperties(cx, proto, SavedFrame::properties) - || !JS_DefineFunctions(cx, proto, SavedFrame::methods) - || !FreezeObject(cx, proto)) - { - return nullptr; - } - - // The only object with the SavedFrame::class_ that doesn't have a source - // should be the prototype. - proto->setReservedSlot(SavedFrame::JSSLOT_SOURCE, NullValue()); - - savedFrameProto.set(proto); - return savedFrameProto; -} - SavedFrame * SavedStacks::createFrameFromLookup(JSContext *cx, SavedFrame::HandleLookup lookup) { - RootedObject proto(cx, getOrCreateSavedFramePrototype(cx)); + RootedGlobalObject global(cx, cx->global()); + assertSameCompartment(cx, global); + + RootedNativeObject proto(cx, GlobalObject::getOrCreateSavedFramePrototype(cx, global)); if (!proto) return nullptr; - assertSameCompartment(cx, proto); - RootedObject global(cx, cx->compartment()->maybeGlobal()); - if (!global) - return nullptr; - - assertSameCompartment(cx, global); - RootedObject frameObj(cx, NewObjectWithGivenProto(cx, &SavedFrame::class_, proto, global)); if (!frameObj) return nullptr; diff --git a/js/src/vm/SavedStacks.h b/js/src/vm/SavedStacks.h index 7ebde362a95..a73fa5d3f1c 100644 --- a/js/src/vm/SavedStacks.h +++ b/js/src/vm/SavedStacks.h @@ -14,16 +14,22 @@ namespace js { +class SavedFrame; +typedef JS::Handle HandleSavedFrame; +typedef JS::MutableHandle MutableHandleSavedFrame; +typedef JS::Rooted RootedSavedFrame; + class SavedFrame : public NativeObject { friend class SavedStacks; public: static const Class class_; static void finalize(FreeOp *fop, JSObject *obj); + static const JSPropertySpec protoAccessors[]; + static const JSFunctionSpec protoFunctions[]; + static const JSFunctionSpec staticFunctions[]; // Prototype methods and properties to be exposed to JS. - static const JSPropertySpec properties[]; - static const JSFunctionSpec methods[]; static bool construct(JSContext *cx, unsigned argc, Value *vp); static bool sourceProperty(JSContext *cx, unsigned argc, Value *vp); static bool lineProperty(JSContext *cx, unsigned argc, Value *vp); @@ -53,6 +59,7 @@ class SavedFrame : public NativeObject { class HandleLookup; private: + static bool finishSavedFrameInit(JSContext *cx, HandleObject ctor, HandleObject proto); void initFromLookup(HandleLookup lookup); enum { @@ -77,16 +84,13 @@ class SavedFrame : public NativeObject { // know that GC moved the parent and we need to update our private value and // rekey the saved frame in its hash set. These two methods are helpers for // this process. - bool parentMoved(); - void updatePrivateParent(); + bool parentMoved(); + void updatePrivateParent(); - static SavedFrame *checkThis(JSContext *cx, CallArgs &args, const char *fnName); + static bool checkThis(JSContext *cx, CallArgs &args, const char *fnName, + MutableHandleSavedFrame frame); }; -typedef JS::Handle HandleSavedFrame; -typedef JS::MutableHandle MutableHandleSavedFrame; -typedef JS::Rooted RootedSavedFrame; - struct SavedFrame::HashPolicy { typedef SavedFrame::Lookup Lookup; @@ -106,7 +110,6 @@ class SavedStacks { public: SavedStacks() : frames(), - savedFrameProto(nullptr), allocationSamplingProbability(1.0), allocationSkipCount(0), // XXX: Initialize the RNG state to 0 so that random_initSeed is lazily @@ -129,7 +132,6 @@ class SavedStacks { private: SavedFrame::Set frames; - ReadBarrieredObject savedFrameProto; double allocationSamplingProbability; uint32_t allocationSkipCount; uint64_t rngState; @@ -137,9 +139,6 @@ class SavedStacks { bool insertFrames(JSContext *cx, FrameIter &iter, MutableHandleSavedFrame frame, unsigned maxFrameCount = 0); SavedFrame *getOrCreateSavedFrame(JSContext *cx, SavedFrame::HandleLookup lookup); - // |SavedFrame.prototype| is created lazily and held weakly. It should only - // be accessed through this method. - JSObject *getOrCreateSavedFramePrototype(JSContext *cx); SavedFrame *createFrameFromLookup(JSContext *cx, SavedFrame::HandleLookup lookup); void chooseSamplingProbability(JSContext* cx); diff --git a/js/xpconnect/src/xpcprivate.h b/js/xpconnect/src/xpcprivate.h index 273c826cb8a..5dea301f4c5 100644 --- a/js/xpconnect/src/xpcprivate.h +++ b/js/xpconnect/src/xpcprivate.h @@ -507,7 +507,7 @@ public: // Mapping of often used strings to jsid atoms that live 'forever'. // // To add a new string: add to this list and to XPCJSRuntime::mStrings - // at the top of xpcjsruntime.cpp + // at the top of XPCJSRuntime.cpp enum { IDX_CONSTRUCTOR = 0 , IDX_TO_STRING , diff --git a/js/xpconnect/tests/unit/test_xray_SavedFrame.js b/js/xpconnect/tests/unit/test_xray_SavedFrame.js new file mode 100644 index 00000000000..6a931cff0ce --- /dev/null +++ b/js/xpconnect/tests/unit/test_xray_SavedFrame.js @@ -0,0 +1,108 @@ +// Bug 1117242: Test calling SavedFrame getters from globals that don't subsume +// that frame's principals. + +const Cc = Components.classes; +const Ci = Components.interfaces; +const Cu = Components.utils; + +Cu.import("resource://gre/modules/jsdebugger.jsm"); +addDebuggerToGlobal(this); + +const lowP = Cc["@mozilla.org/nullprincipal;1"].createInstance(Ci.nsIPrincipal); +const midP = [lowP, "http://other.com"]; +const highP = Cc["@mozilla.org/systemprincipal;1"].createInstance(Ci.nsIPrincipal); + +const low = new Cu.Sandbox(lowP); +const mid = new Cu.Sandbox(midP); +const high = new Cu.Sandbox(highP); + +function run_test() { + // Test that the priveleged view of a SavedFrame from a subsumed compartment + // is the same view that the subsumed compartment gets. Create the following + // chain of function calls (with some intermediate system-principaled frames + // due to implementation): + // + // low.lowF -> mid.midF -> high.highF -> high.saveStack + // + // Where high.saveStack gets monkey patched to create stacks in each of our + // sandboxes. + + Cu.evalInSandbox("function highF() { return saveStack(); }", high); + + mid.highF = () => high.highF(); + Cu.evalInSandbox("function midF() { return highF(); }", mid); + + low.midF = () => mid.midF(); + Cu.evalInSandbox("function lowF() { return midF(); }", low); + + const expected = [ + { + sandbox: low, + frames: ["lowF"], + }, + { + sandbox: mid, + frames: ["midF", "lowF"], + }, + { + sandbox: high, + frames: ["getSavedFrameInstanceFromSandbox", + "saveStack", + "highF", + "run_test/mid.highF", + "midF", + "run_test/low.midF", + "lowF", + "run_test", + "_execute_test", + null], + } + ]; + + for (let { sandbox, frames } of expected) { + high.saveStack = function saveStack() { + return getSavedFrameInstanceFromSandbox(sandbox); + }; + + const xrayStack = low.lowF(); + equal(xrayStack.functionDisplayName, "getSavedFrameInstanceFromSandbox", + "Xrays should always be able to see everything."); + + let waived = Cu.waiveXrays(xrayStack); + do { + ok(frames.length, + "There should still be more expected frames while we have actual frames."); + equal(waived.functionDisplayName, frames.shift(), + "The waived wrapper should give us the stack's compartment's view."); + waived = waived.parent; + } while (waived); + } +} + +// Get a SavedFrame instance from inside the given sandbox. +// +// We can't use Cu.getJSTestingFunctions().saveStack() because Cu isn't +// available to sandboxes that don't have the system principal. The easiest way +// to get the SavedFrame is to use the Debugger API to track allocation sites +// and then do an allocation. +function getSavedFrameInstanceFromSandbox(sandbox) { + const dbg = new Debugger(sandbox); + + dbg.memory.trackingAllocationSites = true; + Cu.evalInSandbox("new Object", sandbox); + const allocs = dbg.memory.drainAllocationsLog(); + dbg.memory.trackingAllocationSites = false; + + ok(allocs[0], "We should observe the allocation"); + const { frame } = allocs[0]; + + if (sandbox !== high) { + ok(Cu.isXrayWrapper(frame), "`frame` should be an xray..."); + equal(Object.prototype.toString.call(Cu.waiveXrays(frame)), + "[object SavedFrame]", + "...and that xray should wrap a SavedFrame"); + } + + return frame; +} + diff --git a/js/xpconnect/tests/unit/xpcshell.ini b/js/xpconnect/tests/unit/xpcshell.ini index 9195ca70912..9dd5fa69876 100644 --- a/js/xpconnect/tests/unit/xpcshell.ini +++ b/js/xpconnect/tests/unit/xpcshell.ini @@ -108,3 +108,4 @@ head = head_watchdog.js head = head_watchdog.js [test_writeToGlobalPrototype.js] [test_xrayed_iterator.js] +[test_xray_SavedFrame.js] \ No newline at end of file diff --git a/js/xpconnect/wrappers/XrayWrapper.cpp b/js/xpconnect/wrappers/XrayWrapper.cpp index a7d11ae12c4..d869d144c1d 100644 --- a/js/xpconnect/wrappers/XrayWrapper.cpp +++ b/js/xpconnect/wrappers/XrayWrapper.cpp @@ -83,6 +83,7 @@ IsJSXraySupported(JSProtoKey key) case JSProto_Array: case JSProto_Function: case JSProto_TypedArray: + case JSProto_SavedFrame: return true; default: return false; diff --git a/xpcom/base/nsIException.idl b/xpcom/base/nsIException.idl index 697adb52a59..902397eaa9d 100644 --- a/xpcom/base/nsIException.idl +++ b/xpcom/base/nsIException.idl @@ -26,6 +26,10 @@ interface nsIStackFrame : nsISupports readonly attribute AUTF8String sourceLine; readonly attribute nsIStackFrame caller; + // Returns the first frame whose principals are subsumed by the caller's + // principals. + [noscript, implicit_jscontext] readonly attribute nsIStackFrame sanitized; + // Returns a formatted stack string that looks like the sort of // string that would be returned by .stack on JS Error objects. // Only works on JS-language stack frames. @@ -59,7 +63,7 @@ interface nsIException : nsISupports readonly attribute AString filename; // Valid line numbers begin at '1'. '0' indicates unknown. readonly attribute uint32_t lineNumber; - // Valid column numbers begin at 0. + // Valid column numbers begin at 0. // We don't have an unambiguous indicator for unknown. readonly attribute uint32_t columnNumber;