Version 2.7.2
* Cherry-pick 5581307ae6ade7a6a093a9643993b159f270ac56 to stable
* Cherry-pick ca694e5ebf194e00644b77eeab31fd093640c5d8 to stable
* Cherry-pick 0c0e041a8d864804bdf7f09a5586a3de93a3259a to stable
* Cherry-pick 4af4245c45184ae41a74d4752f4ec819f387943f to stable
* Cherry-pick 6a5d73dfe7acc91249a37a4f80207da0da5f211b to stable
* Cherry-pick c4e526d7d8ba8352132192d3fbcd69ac2844c441 to stable
diff --git a/CHANGELOG.md b/CHANGELOG.md
index d316f79..5df7170 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,4 +1,17 @@
-## 2.7.1 - 2020-01-32
+## 2.7.2 - 2020-03-23
+
+This is a patch release that addresses a vulnerability dart:html
+[NodeValidator](https://api.dart.dev/stable/dart-html/NodeValidator-class.html)
+related to DOM clobbering of `previousSibling`. Thanks to **Vincenzo di Cicco**
+for finding and reporting this issue.
+
+This release also improves compatibility with ARMv8 processors
+(issue [40001][]) and dart:io stability (issue [40589][]).
+
+[40001]: https://github.com/dart-lang/sdk/issues/40001
+[40589]: https://github.com/dart-lang/sdk/issues/40589
+
+## 2.7.1 - 2020-01-23
This is a patch release that improves dart2js compile-time (issue [40217][]).
diff --git a/DEPS b/DEPS
index 8583d7a..6eacf04 100644
--- a/DEPS
+++ b/DEPS
@@ -276,8 +276,6 @@
Var("dart_git") + "dart_style.git" + "@" + Var("dart_style_tag"),
Var("dart_root") + "/third_party/pkg/dart2js_info":
Var("dart_git") + "dart2js_info.git" + "@" + Var("dart2js_info_tag"),
- Var("dart_root") + "/third_party/pkg/args":
- Var("dart_git") + "args.git" + "@" + Var("args_tag"),
Var("dart_root") + "/third_party/pkg/dartdoc":
Var("dart_git") + "dartdoc.git" + "@" + Var("dartdoc_tag"),
Var("dart_root") + "/third_party/pkg/ffi":
diff --git a/runtime/vm/cpu_arm.cc b/runtime/vm/cpu_arm.cc
index 4078722..2ee57f4 100644
--- a/runtime/vm/cpu_arm.cc
+++ b/runtime/vm/cpu_arm.cc
@@ -172,7 +172,8 @@
// Raspberry Pi, etc.
arm_version_ = ARMv6;
} else if (CpuInfo::FieldContains(kCpuInfoProcessor, "ARMv7") ||
- CpuInfo::FieldContains(kCpuInfoModel, "ARMv7")) {
+ CpuInfo::FieldContains(kCpuInfoModel, "ARMv7") ||
+ CpuInfo::FieldContains(kCpuInfoArchitecture, "7")) {
arm_version_ = ARMv7;
} else {
#if defined(DART_RUN_IN_QEMU_ARMv7)
diff --git a/sdk/lib/_internal/vm/bin/socket_patch.dart b/sdk/lib/_internal/vm/bin/socket_patch.dart
index 5771c76..b499efa 100644
--- a/sdk/lib/_internal/vm/bin/socket_patch.dart
+++ b/sdk/lib/_internal/vm/bin/socket_patch.dart
@@ -706,32 +706,52 @@
String get _serviceTypePath => throw new UnimplementedError();
String get _serviceTypeName => throw new UnimplementedError();
- Uint8List read(int len) {
- if (len != null && len <= 0) {
- throw new ArgumentError("Illegal length $len");
+ Uint8List read(int count) {
+ if (count != null && count <= 0) {
+ throw ArgumentError("Illegal length $count");
}
if (isClosing || isClosed) return null;
- len = min(available, len == null ? available : len);
- if (len == 0) return null;
- var result = nativeRead(len);
- if (result is OSError) {
- reportError(result, StackTrace.current, "Read failed");
- return null;
- }
- if (result != null) {
- available -= result.length;
- // TODO(ricow): Remove when we track internal and pipe uses.
- assert(resourceInfo != null || isPipe || isInternal || isInternalSignal);
- if (resourceInfo != null) {
- resourceInfo.totalRead += result.length;
+ var list;
+ if (count != null) {
+ list = nativeRead(count);
+ if (list is OSError) {
+ reportError(list, StackTrace.current, "Read failed");
+ return null;
+ }
+ } else {
+ // If count is null, read as many bytes as possible.
+ // Loop here to ensure bytes that arrived while this read was
+ // issued are also read.
+ BytesBuilder builder = BytesBuilder();
+ do {
+ assert(available > 0);
+ list = nativeRead(available);
+ if (list is OSError) {
+ reportError(list, StackTrace.current, "Read failed");
+ return null;
+ }
+ if (list == null) {
+ break;
+ }
+ builder.add(list);
+ available = nativeAvailable();
+ } while (available > 0);
+ if (builder.isEmpty) {
+ list = null;
+ } else {
+ list = builder.toBytes();
}
}
- // TODO(ricow): Remove when we track internal and pipe uses.
- assert(resourceInfo != null || isPipe || isInternal || isInternalSignal);
+ if (list != null) {
+ if (resourceInfo != null) {
+ resourceInfo.totalRead += list.length;
+ }
+ }
if (resourceInfo != null) {
resourceInfo.didRead();
}
- return result;
+ available = nativeAvailable();
+ return list;
}
Datagram receive() {
diff --git a/sdk/lib/html/dart2js/html_dart2js.dart b/sdk/lib/html/dart2js/html_dart2js.dart
index bdf8bac..87cab0f 100644
--- a/sdk/lib/html/dart2js/html_dart2js.dart
+++ b/sdk/lib/html/dart2js/html_dart2js.dart
@@ -13204,6 +13204,14 @@
if (!(element.attributes instanceof NamedNodeMap)) {
return true;
}
+ // If something has corrupted the traversal we want to detect
+ // these on not only the children (tested below) but on the node itself
+ // in case it was bypassed.
+ if (element["id"] == 'lastChild' || element["name"] == 'lastChild' ||
+ element["id"] == 'previousSibling' || element["name"] == 'previousSibling' ||
+ element["id"] == 'children' || element["name"] == 'children') {
+ return true;
+ }
var childNodes = element.childNodes;
if (element.lastChild &&
element.lastChild !== childNodes[childNodes.length -1]) {
@@ -13229,6 +13237,7 @@
// allowing us to check for clobbering that may show up in other accesses.
if (child["id"] == 'attributes' || child["name"] == 'attributes' ||
child["id"] == 'lastChild' || child["name"] == 'lastChild' ||
+ child["id"] == 'previousSibling' || child["name"] == 'previousSibling' ||
child["id"] == 'children' || child["name"] == 'children') {
return true;
}
@@ -39336,6 +39345,9 @@
*/
class _ValidatingTreeSanitizer implements NodeTreeSanitizer {
NodeValidator validator;
+
+ /// Did we modify the tree by removing anything.
+ bool modifiedTree = false;
_ValidatingTreeSanitizer(this.validator) {}
void sanitizeTree(Node node) {
@@ -39346,9 +39358,13 @@
while (null != child) {
var nextChild;
try {
- // Child may be removed during the walk, and we may not
- // even be able to get its previousNode.
+ // Child may be removed during the walk, and we may not even be able
+ // to get its previousNode. But it's also possible that previousNode
+ // (i.e. previousSibling) is being spoofed, so double-check it.
nextChild = child.previousNode;
+ if (nextChild != null && nextChild.nextNode != child) {
+ throw StateError("Corrupt HTML");
+ }
} catch (e) {
// Child appears bad, remove it. We want to check the rest of the
// children of node and, but we have no way of getting to the next
@@ -39362,7 +39378,12 @@
}
}
+ modifiedTree = false;
walk(node, null);
+ while (modifiedTree) {
+ modifiedTree = false;
+ walk(node, null);
+ }
}
/// Aggressively try to remove node.
@@ -39370,7 +39391,8 @@
// If we have the parent, it's presumably already passed more sanitization
// or is the fragment, so ask it to remove the child. And if that fails
// try to set the outer html.
- if (parent == null) {
+ modifiedTree = true;
+ if (parent == null || parent != node.parentNode) {
node.remove();
} else {
parent._removeChild(node);
diff --git a/sdk_nnbd/lib/_internal/vm/bin/socket_patch.dart b/sdk_nnbd/lib/_internal/vm/bin/socket_patch.dart
index 750aca8..689e88d 100644
--- a/sdk_nnbd/lib/_internal/vm/bin/socket_patch.dart
+++ b/sdk_nnbd/lib/_internal/vm/bin/socket_patch.dart
@@ -706,30 +706,55 @@
String get _serviceTypePath => throw new UnimplementedError();
String get _serviceTypeName => throw new UnimplementedError();
- Uint8List read(int len) {
- if (len != null && len <= 0) {
- throw new ArgumentError("Illegal length $len");
+ Uint8List? read(int? count) {
+ if (count != null && count <= 0) {
+ throw ArgumentError("Illegal length $count");
}
if (isClosing || isClosed) return null;
- len = min(available, len == null ? available : len);
- if (len == 0) return null;
- var result = nativeRead(len);
- if (result is OSError) {
- reportError(result, StackTrace.current, "Read failed");
- return null;
- }
- if (result != null) {
- available -= result.length;
- // TODO(ricow): Remove when we track internal and pipe uses.
- assert(resourceInfo != null || isPipe || isInternal || isInternalSignal);
- if (resourceInfo != null) {
- resourceInfo.totalRead += result.length;
+ var list;
+ if (count != null) {
+ list = nativeRead(count);
+ if (list is OSError) {
+ reportError(list, StackTrace.current, "Read failed");
+ return null;
+ }
+ } else {
+ // If count is null, read as many bytes as possible.
+ // Loop here to ensure bytes that arrived while this read was
+ // issued are also read.
+ BytesBuilder builder = BytesBuilder();
+ do {
+ assert(available > 0);
+ list = nativeRead(available);
+ if (list is OSError) {
+ reportError(list, StackTrace.current, "Read failed");
+ return null;
+ }
+ if (list == null) {
+ break;
+ }
+ builder.add(list);
+ available = nativeAvailable();
+ } while (available > 0);
+ if (builder.isEmpty) {
+ list = null;
+ } else {
+ list = builder.toBytes();
}
}
- // TODO(ricow): Remove when we track internal and pipe uses.
- assert(resourceInfo != null || isPipe || isInternal || isInternalSignal);
- if (resourceInfo != null) {
- resourceInfo.didRead();
+ final result = list as Uint8List?;
+ final resourceInformation = resourceInfo;
+ assert(resourceInformation != null ||
+ isPipe ||
+ isInternal ||
+ isInternalSignal);
+ if (result != null) {
+ if (resourceInformation != null) {
+ resourceInformation.totalRead += result.length;
+ }
+ }
+ if (resourceInformation != null) {
+ resourceInformation.didRead();
}
return result;
}
diff --git a/sdk_nnbd/lib/html/dart2js/html_dart2js.dart b/sdk_nnbd/lib/html/dart2js/html_dart2js.dart
index c7913ec..d1718a6 100644
--- a/sdk_nnbd/lib/html/dart2js/html_dart2js.dart
+++ b/sdk_nnbd/lib/html/dart2js/html_dart2js.dart
@@ -13203,6 +13203,14 @@
if (!(element.attributes instanceof NamedNodeMap)) {
return true;
}
+ // If something has corrupted the traversal we want to detect
+ // these on not only the children (tested below) but on the node itself
+ // in case it was bypassed.
+ if (element["id"] == 'lastChild' || element["name"] == 'lastChild' ||
+ element["id"] == 'previousSibling' || element["name"] == 'previousSibling' ||
+ element["id"] == 'children' || element["name"] == 'children') {
+ return true;
+ }
var childNodes = element.childNodes;
if (element.lastChild &&
element.lastChild !== childNodes[childNodes.length -1]) {
@@ -13228,6 +13236,7 @@
// allowing us to check for clobbering that may show up in other accesses.
if (child["id"] == 'attributes' || child["name"] == 'attributes' ||
child["id"] == 'lastChild' || child["name"] == 'lastChild' ||
+ child["id"] == 'previousSibling' || child["name"] == 'previousSibling' ||
child["id"] == 'children' || child["name"] == 'children') {
return true;
}
@@ -39335,6 +39344,9 @@
*/
class _ValidatingTreeSanitizer implements NodeTreeSanitizer {
NodeValidator validator;
+
+ /// Did we modify the tree by removing anything.
+ bool modifiedTree = false;
_ValidatingTreeSanitizer(this.validator) {}
void sanitizeTree(Node node) {
@@ -39345,9 +39357,13 @@
while (null != child) {
var nextChild;
try {
- // Child may be removed during the walk, and we may not
- // even be able to get its previousNode.
+ // Child may be removed during the walk, and we may not even be able
+ // to get its previousNode. But it's also possible that previousNode
+ // (i.e. previousSibling) is being spoofed, so double-check it.
nextChild = child.previousNode;
+ if (nextChild != null && nextChild.nextNode != child) {
+ throw StateError("Corrupt HTML");
+ }
} catch (e) {
// Child appears bad, remove it. We want to check the rest of the
// children of node and, but we have no way of getting to the next
@@ -39361,7 +39377,12 @@
}
}
+ modifiedTree = false;
walk(node, null);
+ while (modifiedTree) {
+ modifiedTree = false;
+ walk(node, null);
+ }
}
/// Aggressively try to remove node.
@@ -39369,7 +39390,8 @@
// If we have the parent, it's presumably already passed more sanitization
// or is the fragment, so ask it to remove the child. And if that fails
// try to set the outer html.
- if (parent == null) {
+ modifiedTree = true;
+ if (parent == null || parent != node.parentNode) {
node.remove();
} else {
parent._removeChild(node);
diff --git a/tests/lib_2/html/node_validator_important_if_you_suppress_make_the_bug_critical_test.dart b/tests/lib_2/html/node_validator_important_if_you_suppress_make_the_bug_critical_test.dart
index 194e43a..1b3ea60 100644
--- a/tests/lib_2/html/node_validator_important_if_you_suppress_make_the_bug_critical_test.dart
+++ b/tests/lib_2/html/node_validator_important_if_you_suppress_make_the_bug_critical_test.dart
@@ -428,6 +428,31 @@
"<form id='single_node_clobbering' onmouseover='alert(1)'><input name='attributes'>",
"");
+ testHtml('DOM clobbering of previousSibling',
+ validator,
+ 'https://example.com/<div><img/src="x"/onerror="alert();"/><form><input/name="previousSibling"></form></div>',
+ 'https://example.com/<div><img src="x"></div>'
+ );
+
+ // The lastChild and childNodes clobbering corrupts iterating the tree. They
+ // get removed, but in the meantime the evil node has been skipped. Note
+ // that we end up deleting the entire form because the clobbering makes its
+ // lastChild incorrect and we detect this, but only on the second time
+ // through.
+ testHtml('DOM clobbering of both childNodes and lastChild',
+ validator,
+ "abc<form><div><img src='x' onerror='alert(document.domain)'></div><div><input name='childNodes'><input id='lastChild' name='childNodes'></div></form>",
+ 'abc'
+ );
+
+ test('DOM clobbering of previousSibling via append', () {
+ var div = Element.div();
+ var bad =
+ 'https://example.com/<div><img/src="x"/onerror="alert();"/><form><input/name="previousSibling"></form></div>';
+ div.innerHtml = bad;
+ expect(div.innerHtml, 'https://example.com/<div><img src="x"></div>');
+ });
+
testHtml(
'DOM clobbering of attributes with multiple nodes',
validator,
diff --git a/tests/standalone/io/socket_hang_test.dart b/tests/standalone/io/socket_hang_test.dart
new file mode 100644
index 0000000..2231844
--- /dev/null
+++ b/tests/standalone/io/socket_hang_test.dart
@@ -0,0 +1,24 @@
+// Copyright (c) 2020, the Dart project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+import 'dart:convert';
+import 'dart:io' as io;
+
+void main(List<String> args) async {
+ if (args.length != 0) {
+ for (int i = 0; i < 100000; i++) {
+ print('line $i');
+ }
+ print('done');
+ return;
+ } else {
+ // Create child process and keeps writing into stdout.
+ final p = await io.Process.start(
+ io.Platform.executable, [io.Platform.script.toFilePath(), 'child']);
+ p.stdout.transform(utf8.decoder).listen((x) => print('stdout: $x'));
+ p.stderr.transform(utf8.decoder).listen((x) => print('stderr: $x'));
+ final exitCode = await p.exitCode;
+ print('process exited with ${exitCode}');
+ }
+}
diff --git a/tests/standalone_2/io/socket_hang_test.dart b/tests/standalone_2/io/socket_hang_test.dart
new file mode 100644
index 0000000..2231844
--- /dev/null
+++ b/tests/standalone_2/io/socket_hang_test.dart
@@ -0,0 +1,24 @@
+// Copyright (c) 2020, the Dart project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+import 'dart:convert';
+import 'dart:io' as io;
+
+void main(List<String> args) async {
+ if (args.length != 0) {
+ for (int i = 0; i < 100000; i++) {
+ print('line $i');
+ }
+ print('done');
+ return;
+ } else {
+ // Create child process and keeps writing into stdout.
+ final p = await io.Process.start(
+ io.Platform.executable, [io.Platform.script.toFilePath(), 'child']);
+ p.stdout.transform(utf8.decoder).listen((x) => print('stdout: $x'));
+ p.stderr.transform(utf8.decoder).listen((x) => print('stderr: $x'));
+ final exitCode = await p.exitCode;
+ print('process exited with ${exitCode}');
+ }
+}
diff --git a/tools/VERSION b/tools/VERSION
index 42b0140..9ccd548 100644
--- a/tools/VERSION
+++ b/tools/VERSION
@@ -32,7 +32,7 @@
CHANNEL stable
MAJOR 2
MINOR 7
-PATCH 1
+PATCH 2
PRERELEASE 0
PRERELEASE_PATCH 0
ABI_VERSION 24
diff --git a/tools/dom/src/Validators.dart b/tools/dom/src/Validators.dart
index ac0663d..a779f87 100644
--- a/tools/dom/src/Validators.dart
+++ b/tools/dom/src/Validators.dart
@@ -156,6 +156,9 @@
*/
class _ValidatingTreeSanitizer implements NodeTreeSanitizer {
NodeValidator validator;
+
+ /// Did we modify the tree by removing anything.
+ bool modifiedTree = false;
_ValidatingTreeSanitizer(this.validator) {}
void sanitizeTree(Node node) {
@@ -166,9 +169,13 @@
while (null != child) {
var nextChild;
try {
- // Child may be removed during the walk, and we may not
- // even be able to get its previousNode.
+ // Child may be removed during the walk, and we may not even be able
+ // to get its previousNode. But it's also possible that previousNode
+ // (i.e. previousSibling) is being spoofed, so double-check it.
nextChild = child.previousNode;
+ if (nextChild != null && nextChild.nextNode != child) {
+ throw StateError("Corrupt HTML");
+ }
} catch (e) {
// Child appears bad, remove it. We want to check the rest of the
// children of node and, but we have no way of getting to the next
@@ -182,7 +189,12 @@
}
}
+ modifiedTree = false;
walk(node, null);
+ while (modifiedTree) {
+ modifiedTree = false;
+ walk(node, null);
+ }
}
/// Aggressively try to remove node.
@@ -190,7 +202,8 @@
// If we have the parent, it's presumably already passed more sanitization
// or is the fragment, so ask it to remove the child. And if that fails
// try to set the outer html.
- if (parent == null) {
+ modifiedTree = true;
+ if (parent == null || parent != node.parentNode) {
node.remove();
} else {
parent._removeChild(node);
diff --git a/tools/dom/templates/html/impl/impl_Element.darttemplate b/tools/dom/templates/html/impl/impl_Element.darttemplate
index 3fd0d4e..5a24280 100644
--- a/tools/dom/templates/html/impl/impl_Element.darttemplate
+++ b/tools/dom/templates/html/impl/impl_Element.darttemplate
@@ -1442,6 +1442,14 @@
if (!(element.attributes instanceof NamedNodeMap)) {
return true;
}
+ // If something has corrupted the traversal we want to detect
+ // these on not only the children (tested below) but on the node itself
+ // in case it was bypassed.
+ if (element["id"] == 'lastChild' || element["name"] == 'lastChild' ||
+ element["id"] == 'previousSibling' || element["name"] == 'previousSibling' ||
+ element["id"] == 'children' || element["name"] == 'children') {
+ return true;
+ }
var childNodes = element.childNodes;
if (element.lastChild &&
element.lastChild !== childNodes[childNodes.length -1]) {
@@ -1467,6 +1475,7 @@
// allowing us to check for clobbering that may show up in other accesses.
if (child["id"] == 'attributes' || child["name"] == 'attributes' ||
child["id"] == 'lastChild' || child["name"] == 'lastChild' ||
+ child["id"] == 'previousSibling' || child["name"] == 'previousSibling' ||
child["id"] == 'children' || child["name"] == 'children') {
return true;
}
