Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (196524 => 196525)
--- trunk/Source/_javascript_Core/ChangeLog 2016-02-13 00:07:04 UTC (rev 196524)
+++ trunk/Source/_javascript_Core/ChangeLog 2016-02-13 00:12:54 UTC (rev 196525)
@@ -1,3 +1,46 @@
+2016-02-12 Saam barati <sbar...@apple.com>
+
+ The parser doesn't properly protect against global variable references in builtins
+ https://bugs.webkit.org/show_bug.cgi?id=154144
+
+ Reviewed by Geoffrey Garen.
+
+ This patch fixes our global variable reference detection
+ algorithm that was broken. After fixing the algorithm, I
+ detected many places where we were incorrectly using global
+ variables. I've fixed all those.
+
+ * builtins/BuiltinExecutables.cpp:
+ (JSC::createExecutableInternal):
+ * builtins/NumberPrototype.js:
+ (toLocaleString):
+ * builtins/PromiseConstructor.js:
+ (race):
+ (reject):
+ (resolve):
+ * parser/Nodes.cpp:
+ (JSC::ProgramNode::ProgramNode):
+ (JSC::ModuleProgramNode::ModuleProgramNode):
+ (JSC::ProgramNode::setClosedVariables): Deleted.
+ * parser/Nodes.h:
+ (JSC::ScopeNode::setClosedVariables): Deleted.
+ (JSC::ProgramNode::closedVariables): Deleted.
+ * parser/Parser.cpp:
+ (JSC::Parser<LexerType>::parseInner):
+ (JSC::Parser<LexerType>::didFinishParsing):
+ * parser/Parser.h:
+ (JSC::Scope::setIsLexicalScope):
+ (JSC::Scope::isLexicalScope):
+ (JSC::Scope::closedVariableCandidates):
+ (JSC::Scope::declaredVariables):
+ (JSC::Scope::lexicalVariables):
+ (JSC::Scope::finalizeLexicalEnvironment):
+ (JSC::Parser::positionBeforeLastNewline):
+ (JSC::Parser::locationBeforeLastToken):
+ (JSC::Parser::isFunctionMetadataNode):
+ (JSC::parse):
+ (JSC::Parser::closedVariables): Deleted.
+
2016-02-12 Filip Pizlo <fpi...@apple.com>
JSObject::putByIndexBeyondVectorLengthWithoutAttributes needs to go to the sparse map based on MAX_STORAGE_VECTOR_INDEX
Modified: trunk/Source/_javascript_Core/builtins/BuiltinExecutables.cpp (196524 => 196525)
--- trunk/Source/_javascript_Core/builtins/BuiltinExecutables.cpp 2016-02-13 00:07:04 UTC (rev 196524)
+++ trunk/Source/_javascript_Core/builtins/BuiltinExecutables.cpp 2016-02-13 00:12:54 UTC (rev 196525)
@@ -106,13 +106,6 @@
// This function assumes an input string that would result in a single anonymous function _expression_.
metadata->setEndPosition(positionBeforeLastNewline);
RELEASE_ASSERT(metadata);
- for (const auto& closedVariable : program->closedVariables()) {
- if (closedVariable == vm.propertyNames->arguments.impl())
- continue;
-
- if (closedVariable == vm.propertyNames->undefinedKeyword.impl())
- continue;
- }
metadata->overrideName(name);
VariableEnvironment dummyTDZVariables;
UnlinkedFunctionExecutable* functionExecutable = UnlinkedFunctionExecutable::create(&vm, source, metadata, kind, constructAbility, dummyTDZVariables, DerivedContextType::None, WTFMove(sourceOverride));
Modified: trunk/Source/_javascript_Core/builtins/NumberPrototype.js (196524 => 196525)
--- trunk/Source/_javascript_Core/builtins/NumberPrototype.js 2016-02-13 00:07:04 UTC (rev 196524)
+++ trunk/Source/_javascript_Core/builtins/NumberPrototype.js 2016-02-13 00:12:54 UTC (rev 196525)
@@ -35,7 +35,7 @@
// 1. Let x be thisNumberValue(this value).
// 2. ReturnIfAbrupt(x).
var number = this;
- if (!(typeof number === "number" || number instanceof Number))
+ if (!(typeof number === "number" || number instanceof @Number))
throw new @TypeError("Number.prototype.toLocaleString called on incompatible " + typeof number);
// 3. Let numberFormat be Construct(%NumberFormat%, «locales, options»).
Modified: trunk/Source/_javascript_Core/builtins/PromiseConstructor.js (196524 => 196525)
--- trunk/Source/_javascript_Core/builtins/PromiseConstructor.js 2016-02-13 00:07:04 UTC (rev 196524)
+++ trunk/Source/_javascript_Core/builtins/PromiseConstructor.js 2016-02-13 00:12:54 UTC (rev 196525)
@@ -28,7 +28,7 @@
"use strict";
if (!@isObject(this))
- throw new TypeError("|this| is not a object");
+ throw new @TypeError("|this| is not a object");
// FIXME: Fix this code when @@species well-known symbol is landed.
// https://bugs.webkit.org/show_bug.cgi?id=146624
@@ -84,7 +84,7 @@
"use strict";
if (!@isObject(this))
- throw new TypeError("|this| is not a object");
+ throw new @TypeError("|this| is not a object");
// FIXME: Fix this code when @@species well-known symbol is landed.
// https://bugs.webkit.org/show_bug.cgi?id=146624
@@ -109,7 +109,7 @@
"use strict";
if (!@isObject(this))
- throw new TypeError("|this| is not a object");
+ throw new @TypeError("|this| is not a object");
var promiseCapability = @newPromiseCapability(this);
@@ -123,7 +123,7 @@
"use strict";
if (!@isObject(this))
- throw new TypeError("|this| is not a object");
+ throw new @TypeError("|this| is not a object");
if (@isPromise(value)) {
var valueConstructor = value.constructor;
Modified: trunk/Source/_javascript_Core/parser/Nodes.cpp (196524 => 196525)
--- trunk/Source/_javascript_Core/parser/Nodes.cpp 2016-02-13 00:07:04 UTC (rev 196524)
+++ trunk/Source/_javascript_Core/parser/Nodes.cpp 2016-02-13 00:12:54 UTC (rev 196525)
@@ -123,11 +123,6 @@
{
}
-void ProgramNode::setClosedVariables(Vector<RefPtr<UniquedStringImpl>>&& closedVariables)
-{
- m_closedVariables = WTFMove(closedVariables);
-}
-
// ------------------------------ ModuleProgramNode -----------------------------
ModuleProgramNode::ModuleProgramNode(ParserArena& parserArena, const JSTokenLocation& startLocation, const JSTokenLocation& endLocation, unsigned startColumn, unsigned endColumn, SourceElements* children, VariableEnvironment& varEnvironment, FunctionStack& funcStack, VariableEnvironment& lexicalVariables, FunctionParameters*, const SourceCode& source, CodeFeatures features, int numConstants)
Modified: trunk/Source/_javascript_Core/parser/Nodes.h (196524 => 196525)
--- trunk/Source/_javascript_Core/parser/Nodes.h 2016-02-13 00:07:04 UTC (rev 196524)
+++ trunk/Source/_javascript_Core/parser/Nodes.h 2016-02-13 00:12:54 UTC (rev 196525)
@@ -1596,8 +1596,6 @@
void emitStatementsBytecode(BytecodeGenerator&, RegisterID* destination);
- void setClosedVariables(Vector<RefPtr<UniquedStringImpl>>&&) { }
-
void analyzeModule(ModuleAnalyzer&);
protected:
@@ -1623,12 +1621,8 @@
static const bool scopeIsFunction = false;
- void setClosedVariables(Vector<RefPtr<UniquedStringImpl>>&&);
- const Vector<RefPtr<UniquedStringImpl>>& closedVariables() const { return m_closedVariables; }
-
private:
virtual void emitBytecode(BytecodeGenerator&, RegisterID* = 0) override;
- Vector<RefPtr<UniquedStringImpl>> m_closedVariables;
unsigned m_startColumn;
unsigned m_endColumn;
};
Modified: trunk/Source/_javascript_Core/parser/Parser.cpp (196524 => 196525)
--- trunk/Source/_javascript_Core/parser/Parser.cpp 2016-02-13 00:07:04 UTC (rev 196524)
+++ trunk/Source/_javascript_Core/parser/Parser.cpp 2016-02-13 00:12:54 UTC (rev 196525)
@@ -300,6 +300,7 @@
bool modifiedParameter = false;
bool modifiedArguments = false;
scope->getCapturedVars(capturedVariables, modifiedParameter, modifiedArguments);
+
VariableEnvironment& varDeclarations = scope->declaredVariables();
for (auto& entry : capturedVariables)
varDeclarations.markVariableAsCaptured(entry);
@@ -321,48 +322,32 @@
if (modifiedArguments)
features |= ModifiedArgumentsFeature;
- Vector<RefPtr<UniquedStringImpl>> closedVariables;
- if (m_parsingBuiltin) {
- // FIXME: This needs to be changed if we want to allow builtins to use lexical declarations.
- for (const auto& variable : usedVariables) {
- Identifier identifier = Identifier::fromUid(m_vm, variable.get());
- if (scope->hasDeclaredVariable(identifier))
- continue;
-
- if (scope->hasDeclaredParameter(identifier))
- continue;
-
- if (variable == m_vm->propertyNames->arguments.impl())
- continue;
-
- closedVariables.append(variable);
- }
-
- if (!capturedVariables.isEmpty()) {
- for (const auto& capturedVariable : capturedVariables) {
- if (scope->hasDeclaredVariable(capturedVariable))
- continue;
-
- if (scope->hasDeclaredParameter(capturedVariable))
- continue;
-
- RELEASE_ASSERT_NOT_REACHED();
+#ifndef NDEBUG
+ if (m_parsingBuiltin && isProgramParseMode(parseMode)) {
+ VariableEnvironment& lexicalVariables = scope->lexicalVariables();
+ const IdentifierSet& closedVariableCandidates = scope->closedVariableCandidates();
+ const BuiltinNames& builtinNames = m_vm->propertyNames->builtinNames();
+ for (const RefPtr<UniquedStringImpl>& candidate : closedVariableCandidates) {
+ if (!lexicalVariables.contains(candidate) && !varDeclarations.contains(candidate) && !builtinNames.isPrivateName(*candidate.get())) {
+ dataLog("Bad global capture in builtin: '", candidate, "'\n");
+ dataLog(m_source->view());
+ CRASH();
}
}
}
- didFinishParsing(sourceElements, context.funcDeclarations(), varDeclarations, features, context.numConstants(), WTFMove(closedVariables));
+#endif // NDEBUG
+ didFinishParsing(sourceElements, context.funcDeclarations(), varDeclarations, features, context.numConstants());
return parseError;
}
template <typename LexerType>
void Parser<LexerType>::didFinishParsing(SourceElements* sourceElements, DeclarationStacks::FunctionStack& funcStack,
- VariableEnvironment& varDeclarations, CodeFeatures features, int numConstants, const Vector<RefPtr<UniquedStringImpl>>&& closedVariables)
+ VariableEnvironment& varDeclarations, CodeFeatures features, int numConstants)
{
m_sourceElements = sourceElements;
m_funcDeclarations.swap(funcStack);
m_varDeclarations.swap(varDeclarations);
- m_closedVariables = closedVariables;
m_features = features;
m_numConstants = numConstants;
}
Modified: trunk/Source/_javascript_Core/parser/Parser.h (196524 => 196525)
--- trunk/Source/_javascript_Core/parser/Parser.h 2016-02-13 00:07:04 UTC (rev 196524)
+++ trunk/Source/_javascript_Core/parser/Parser.h 2016-02-13 00:12:54 UTC (rev 196525)
@@ -293,6 +293,7 @@
}
bool isLexicalScope() { return m_isLexicalScope; }
+ const IdentifierSet& closedVariableCandidates() const { return m_closedVariableCandidates; }
VariableEnvironment& declaredVariables() { return m_declaredVariables; }
VariableEnvironment& lexicalVariables() { return m_lexicalVariables; }
VariableEnvironment& finalizeLexicalEnvironment()
@@ -682,7 +683,6 @@
JSTextPosition positionBeforeLastNewline() const { return m_lexer->positionBeforeLastNewline(); }
JSTokenLocation locationBeforeLastToken() const { return m_lexer->lastTokenLocation(); }
- Vector<RefPtr<UniquedStringImpl>>&& closedVariables() { return WTFMove(m_closedVariables); }
private:
struct AllowInOverride {
@@ -995,7 +995,7 @@
Parser();
String parseInner(const Identifier&, SourceParseMode);
- void didFinishParsing(SourceElements*, DeclarationStacks::FunctionStack&, VariableEnvironment&, CodeFeatures, int, const Vector<RefPtr<UniquedStringImpl>>&&);
+ void didFinishParsing(SourceElements*, DeclarationStacks::FunctionStack&, VariableEnvironment&, CodeFeatures, int);
// Used to determine type of error to report.
bool isFunctionMetadataNode(ScopeNode*) { return false; }
@@ -1407,7 +1407,6 @@
ThisTDZMode m_thisTDZMode;
VariableEnvironment m_varDeclarations;
DeclarationStacks::FunctionStack m_funcDeclarations;
- Vector<RefPtr<UniquedStringImpl>> m_closedVariables;
CodeFeatures m_features;
int m_numConstants;
ExpressionErrorClassifier* m_expressionErrorClassifier;
@@ -1542,8 +1541,6 @@
if (builtinMode == JSParserBuiltinMode::Builtin) {
if (!result)
WTF::dataLog("Error compiling builtin: ", error.message(), "\n");
- else
- result->setClosedVariables(parser.closedVariables());
}
return result;
}
Modified: trunk/Source/WebCore/ChangeLog (196524 => 196525)
--- trunk/Source/WebCore/ChangeLog 2016-02-13 00:07:04 UTC (rev 196524)
+++ trunk/Source/WebCore/ChangeLog 2016-02-13 00:12:54 UTC (rev 196525)
@@ -1,3 +1,33 @@
+2016-02-12 Saam barati <sbar...@apple.com>
+
+ The parser doesn't properly protect against global variable references in builtins
+ https://bugs.webkit.org/show_bug.cgi?id=154144
+
+ Reviewed by Geoffrey Garen.
+
+ Change JS builtins to no longer reference global variables.
+
+ No new tests because old tests cover the issues here.
+
+ * Modules/mediastream/NavigatorUserMedia.js:
+ (webkitGetUserMedia):
+ * Modules/mediastream/RTCPeerConnection.js:
+ (addIceCandidate):
+ (getStats):
+ * Modules/mediastream/RTCPeerConnectionInternals.js:
+ (setLocalOrRemoteDescription):
+ * Modules/plugins/QuickTimePluginReplacement.js:
+ (Replacement.prototype.handleEvent):
+ * Modules/streams/ByteLengthQueuingStrategy.js:
+ (initializeByteLengthQueuingStrategy):
+ * Modules/streams/CountQueuingStrategy.js:
+ (initializeCountQueuingStrategy):
+ * Modules/streams/ReadableStreamInternals.js:
+ (teeReadableStream):
+ * bindings/js/JSDOMGlobalObject.cpp:
+ (WebCore::JSDOMGlobalObject::addBuiltinGlobals):
+ * bindings/js/WebCoreBuiltinNames.h:
+
2016-02-12 Jiewen Tan <jiewen_...@apple.com>
WebKit should expose the DOM 4 Event.isTrusted property
Modified: trunk/Source/WebCore/Modules/mediastream/NavigatorUserMedia.js (196524 => 196525)
--- trunk/Source/WebCore/Modules/mediastream/NavigatorUserMedia.js 2016-02-13 00:07:04 UTC (rev 196524)
+++ trunk/Source/WebCore/Modules/mediastream/NavigatorUserMedia.js 2016-02-13 00:12:54 UTC (rev 196525)
@@ -36,7 +36,7 @@
if (arguments.length < 3)
throw new @TypeError("Not enough arguments");
- if (options !== Object(options))
+ if (options !== @Object(options))
throw new @TypeError("Argument 1 (options) to Navigator.webkitGetUserMedia must be an object");
if (typeof successCallback !== "function")
Modified: trunk/Source/WebCore/Modules/mediastream/RTCPeerConnection.js (196524 => 196525)
--- trunk/Source/WebCore/Modules/mediastream/RTCPeerConnection.js 2016-02-13 00:07:04 UTC (rev 196524)
+++ trunk/Source/WebCore/Modules/mediastream/RTCPeerConnection.js 2016-02-13 00:12:54 UTC (rev 196525)
@@ -68,7 +68,7 @@
throw new @TypeError("Not enough arguments");
var candidate = arguments[0];
- if (!(candidate instanceof RTCIceCandidate))
+ if (!(candidate instanceof @RTCIceCandidate))
throw new @TypeError("Argument 1 ('candidate') to RTCPeerConnection.addIceCandidate must be an instance of RTCIceCandidate");
if (arguments.length == 1) {
@@ -99,7 +99,7 @@
if (arguments.length) {
selector = arguments[0];
- if (selector != null && !(selector instanceof MediaStreamTrack))
+ if (selector != null && !(selector instanceof @MediaStreamTrack))
throw new @TypeError("Argument 1 ('selector') to RTCPeerConnection.getStats must be an instance of MediaStreamTrack");
}
Modified: trunk/Source/WebCore/Modules/mediastream/RTCPeerConnectionInternals.js (196524 => 196525)
--- trunk/Source/WebCore/Modules/mediastream/RTCPeerConnectionInternals.js 2016-02-13 00:07:04 UTC (rev 196524)
+++ trunk/Source/WebCore/Modules/mediastream/RTCPeerConnectionInternals.js 2016-02-13 00:12:54 UTC (rev 196525)
@@ -93,7 +93,7 @@
throw new @TypeError("Not enough arguments");
var description = args[0];
- if (!(description instanceof RTCSessionDescription))
+ if (!(description instanceof @RTCSessionDescription))
throw new @TypeError("Argument 1 ('description') to RTCPeerConnection." + functionName + " must be an instance of RTCSessionDescription");
if (args.length == 1) {
Modified: trunk/Source/WebCore/Modules/plugins/QuickTimePluginReplacement.js (196524 => 196525)
--- trunk/Source/WebCore/Modules/plugins/QuickTimePluginReplacement.js 2016-02-13 00:07:04 UTC (rev 196524)
+++ trunk/Source/WebCore/Modules/plugins/QuickTimePluginReplacement.js 2016-02-13 00:12:54 UTC (rev 196525)
@@ -157,7 +157,7 @@
if (!eventData)
return;
- if (this[eventData] && this[eventData] instanceof Function)
+ if (this[eventData] && typeof this[eventData] === "function")
this[eventData].call(this, event);
else
this.postEvent(eventData);
Modified: trunk/Source/WebCore/Modules/streams/ByteLengthQueuingStrategy.js (196524 => 196525)
--- trunk/Source/WebCore/Modules/streams/ByteLengthQueuingStrategy.js 2016-02-13 00:07:04 UTC (rev 196524)
+++ trunk/Source/WebCore/Modules/streams/ByteLengthQueuingStrategy.js 2016-02-13 00:12:54 UTC (rev 196525)
@@ -37,7 +37,7 @@
{
"use strict";
- Object.defineProperty(this, "highWaterMark", {
+ @Object.defineProperty(this, "highWaterMark", {
value: parameters.highWaterMark,
configurable: true,
enumerable: true,
Modified: trunk/Source/WebCore/Modules/streams/CountQueuingStrategy.js (196524 => 196525)
--- trunk/Source/WebCore/Modules/streams/CountQueuingStrategy.js 2016-02-13 00:07:04 UTC (rev 196524)
+++ trunk/Source/WebCore/Modules/streams/CountQueuingStrategy.js 2016-02-13 00:12:54 UTC (rev 196525)
@@ -36,7 +36,7 @@
{
"use strict";
- Object.defineProperty(this, "highWaterMark", {
+ @Object.defineProperty(this, "highWaterMark", {
value: parameters.highWaterMark,
configurable: true,
enumerable: true,
Modified: trunk/Source/WebCore/Modules/streams/ReadableStreamInternals.js (196524 => 196525)
--- trunk/Source/WebCore/Modules/streams/ReadableStreamInternals.js 2016-02-13 00:07:04 UTC (rev 196524)
+++ trunk/Source/WebCore/Modules/streams/ReadableStreamInternals.js 2016-02-13 00:12:54 UTC (rev 196525)
@@ -87,11 +87,11 @@
const pullFunction = @teeReadableStreamPullFunction(teeState, reader, shouldClone);
- const branch1 = new ReadableStream({
+ const branch1 = new @ReadableStream({
"pull": pullFunction,
"cancel": @teeReadableStreamBranch1CancelFunction(teeState, stream)
});
- const branch2 = new ReadableStream({
+ const branch2 = new @ReadableStream({
"pull": pullFunction,
"cancel": @teeReadableStreamBranch2CancelFunction(teeState, stream)
});
Modified: trunk/Source/WebCore/bindings/js/JSDOMGlobalObject.cpp (196524 => 196525)
--- trunk/Source/WebCore/bindings/js/JSDOMGlobalObject.cpp 2016-02-13 00:07:04 UTC (rev 196524)
+++ trunk/Source/WebCore/bindings/js/JSDOMGlobalObject.cpp 2016-02-13 00:12:54 UTC (rev 196525)
@@ -30,6 +30,10 @@
#include "Document.h"
#include "JSDOMWindow.h"
#include "JSEventListener.h"
+#include "JSMediaStreamTrack.h"
+#include "JSRTCIceCandidate.h"
+#include "JSRTCSessionDescription.h"
+#include "JSReadableStream.h"
#include "JSReadableStreamPrivateConstructors.h"
#include "JSWorkerGlobalScope.h"
#include "WebCoreJSClientData.h"
@@ -78,8 +82,12 @@
JSDOMGlobalObject::GlobalPropertyInfo(clientData.builtinNames().streamReadablePrivateName(), jsNumber(4), DontDelete | ReadOnly),
JSDOMGlobalObject::GlobalPropertyInfo(clientData.builtinNames().streamWaitingPrivateName(), jsNumber(5), DontDelete | ReadOnly),
JSDOMGlobalObject::GlobalPropertyInfo(clientData.builtinNames().streamWritablePrivateName(), jsNumber(6), DontDelete | ReadOnly),
+ JSDOMGlobalObject::GlobalPropertyInfo(clientData.builtinNames().MediaStreamTrackPrivateName(), JSMediaStreamTrack::getConstructor(vm, this), DontDelete | ReadOnly),
+ JSDOMGlobalObject::GlobalPropertyInfo(clientData.builtinNames().ReadableStreamPrivateName(), JSReadableStream::getConstructor(vm, this), DontDelete | ReadOnly),
JSDOMGlobalObject::GlobalPropertyInfo(clientData.builtinNames().ReadableStreamControllerPrivateName(), privateReadableStreamControllerConstructor, DontDelete | ReadOnly),
JSDOMGlobalObject::GlobalPropertyInfo(clientData.builtinNames().ReadableStreamReaderPrivateName(), privateReadableStreamReaderConstructor, DontDelete | ReadOnly),
+ JSDOMGlobalObject::GlobalPropertyInfo(clientData.builtinNames().RTCIceCandidatePrivateName(), JSRTCIceCandidate::getConstructor(vm, this), DontDelete | ReadOnly),
+ JSDOMGlobalObject::GlobalPropertyInfo(clientData.builtinNames().RTCSessionDescriptionPrivateName(), JSRTCSessionDescription::getConstructor(vm, this), DontDelete | ReadOnly),
};
addStaticGlobals(staticGlobals, WTF_ARRAY_LENGTH(staticGlobals));
#endif
Modified: trunk/Source/WebCore/bindings/js/WebCoreBuiltinNames.h (196524 => 196525)
--- trunk/Source/WebCore/bindings/js/WebCoreBuiltinNames.h 2016-02-13 00:07:04 UTC (rev 196524)
+++ trunk/Source/WebCore/bindings/js/WebCoreBuiltinNames.h 2016-02-13 00:12:54 UTC (rev 196525)
@@ -68,8 +68,12 @@
macro(underlyingSink) \
macro(underlyingSource) \
macro(writing) \
+ macro(MediaStreamTrack) \
+ macro(ReadableStream) \
macro(ReadableStreamReader) \
macro(ReadableStreamController) \
+ macro(RTCIceCandidate) \
+ macro(RTCSessionDescription) \
class WebCoreBuiltinNames {
public:
