Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(10)

Unified Diff: sdk/lib/html/dart2js/html_dart2js.dart

Side-by-side diff isn't available for this file because of its large size.
Issue 1365093003: Fix node_validator xss tests for IE and Firefox (Closed) Base URL: https://github.com/dart-lang/sdk.git@master
Patch Set: Created 5 years, 3 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
Download patch
« no previous file with comments | « no previous file | sdk/lib/html/dartium/html_dartium.dart » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: sdk/lib/html/dart2js/html_dart2js.dart
diff --git a/sdk/lib/html/dart2js/html_dart2js.dart b/sdk/lib/html/dart2js/html_dart2js.dart
index ece22d30944a33fa4abd59eb2bd24f11f3823bfd..ba060bb2db26880bb0c1f5544dc7f9a2536809c7 100644
--- a/sdk/lib/html/dart2js/html_dart2js.dart
+++ b/sdk/lib/html/dart2js/html_dart2js.dart
@@ -13328,15 +13328,33 @@ class Element extends Node implements GlobalEventHandlers, ParentNode, ChildNode
return true;
}
}
+ var length = 0;
+ if (element.children) {
+ length = element.children.length;
+ }
+ for (var i = 0; i < length; i++) {
+ var child = element.children[i];
+ // On IE it seems like we sometimes don't see the clobbered attribute,
+ // perhaps as a result of an over-optimization. Also use another route
+ // to check of attributes, children, or lastChild are clobbered. It may
+ // seem silly to check children as we rely on children to do this iteration,
+ // but it seems possible that the access to children might see the real thing,
+ // 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"] == 'children' || child["name"] == 'children') {
+ return true;
+ }
+ }
return false;
})(#)''', element);
}
- String get _safeTagName {
+ static String _safeTagName(element) {
String result = 'element tag unavailable';
try {
- if (tagName is String) {
- result = tagName;
+ if (element.tagName is String) {
+ result = element.tagName;
}
} catch (e) {}
return result;
@@ -37931,11 +37949,11 @@ class _Html5NodeValidator implements NodeValidator {
}
bool allowsElement(Element element) {
- return _allowedElements.contains(element._safeTagName);
+ return _allowedElements.contains(Element._safeTagName(element));
}
bool allowsAttribute(Element element, String attributeName, String value) {
- var tagName = element._safeTagName;
+ var tagName = Element._safeTagName(element);
var validator = _attributeValidators['$tagName::$attributeName'];
if (validator == null) {
validator = _attributeValidators['*::$attributeName'];
@@ -39597,11 +39615,11 @@ class _SimpleNodeValidator implements NodeValidator {
}
bool allowsElement(Element element) {
- return allowedElements.contains(element._safeTagName);
+ return allowedElements.contains(Element._safeTagName(element));
}
bool allowsAttribute(Element element, String attributeName, String value) {
- var tagName = element._safeTagName;
+ var tagName = Element._safeTagName(element);
if (allowedUriAttributes.contains('$tagName::$attributeName')) {
return uriPolicy.allowsUri(value);
} else if (allowedUriAttributes.contains('*::$attributeName')) {
@@ -39642,10 +39660,10 @@ class _CustomElementNodeValidator extends _SimpleNodeValidator {
var isAttr = element.attributes['is'];
if (isAttr != null) {
return allowedElements.contains(isAttr.toUpperCase()) &&
- allowedElements.contains(element._safeTagName);
+ allowedElements.contains(Element._safeTagName(element));
}
}
- return allowCustomTag && allowedElements.contains(element._safeTagName);
+ return allowCustomTag && allowedElements.contains(Element._safeTagName(element));
}
bool allowsAttribute(Element element, String attributeName, String value) {
@@ -39701,7 +39719,7 @@ class _SvgNodeValidator implements NodeValidator {
// foreignobject tag as SvgElement. We don't want foreignobject contents
// anyway, so just remove the whole tree outright. And we can't rely
// on IE recognizing the SvgForeignObject type, so go by tagName. Bug 23144
- if (element is svg.SvgElement && element._safeTagName == 'foreignObject') {
+ if (element is svg.SvgElement && Element._safeTagName(element) == 'foreignObject') {
return false;
}
if (element is svg.SvgElement) {
@@ -40828,14 +40846,14 @@ class _ThrowsNodeValidator implements NodeValidator {
bool allowsElement(Element element) {
if (!validator.allowsElement(element)) {
- throw new ArgumentError(element._safeTagName);
+ throw new ArgumentError(Element._safeTagName(element));
}
return true;
}
bool allowsAttribute(Element element, String attributeName, String value) {
if (!validator.allowsAttribute(element, attributeName, value)) {
- throw new ArgumentError('${element._safeTagName}[$attributeName="$value"]');
+ throw new ArgumentError('${Element._safeTagName(element)}[$attributeName="$value"]');
}
}
}
@@ -40877,7 +40895,7 @@ class _ValidatingTreeSanitizer implements NodeTreeSanitizer {
}
/// Sanitize the element, assuming we can't trust anything about it.
- void _sanitizeUntrustedElement(Element element, Node parent) {
+ void _sanitizeUntrustedElement(/* Element */ element, Node parent) {
// If the _hasCorruptedAttributes does not successfully return false,
// then we consider it corrupted and remove.
// TODO(alanknight): This is a workaround because on Firefox
@@ -40886,7 +40904,9 @@ class _ValidatingTreeSanitizer implements NodeTreeSanitizer {
// can't call methods. This does mean that you can't explicitly allow an
// embed tag. The only thing that will let it through is a null
// sanitizer that doesn't traverse the tree at all. But sanitizing while
- // allowing embeds seems quite unlikely.
+ // allowing embeds seems quite unlikely. This is also the reason that we
+ // can't declare the type of element, as an embed won't pass any type
+ // check in dart2js.
var corrupted = true;
var attrs;
var isAttr;
@@ -40894,15 +40914,27 @@ class _ValidatingTreeSanitizer implements NodeTreeSanitizer {
// If getting/indexing attributes throws, count that as corrupt.
attrs = element.attributes;
isAttr = attrs['is'];
- corrupted = Element._hasCorruptedAttributes(element);
+ // On IE, erratically, the hasCorruptedAttributes test can return false,
+ // even though it clearly is corrupted. A separate copy of the test
+ // inlining just the basic check seems to help.
+ var corruptedTest1 = Element._hasCorruptedAttributes(element);
+ var corruptedTest2 = JS('bool', r'!(#.attributes instanceof NamedNodeMap)', element);
+ corrupted = corruptedTest1 || corruptedTest2;
} catch(e) {}
- var elementText = 'element unprintable';
+ var elementText = 'element unprintable';
try {
elementText = element.toString();
} catch(e) {}
- var elementTagName = element._safeTagName;
- _sanitizeElement(element, parent, corrupted, elementText, elementTagName,
- attrs, isAttr);
+ try {
+ var elementTagName = Element._safeTagName(element);
+ _sanitizeElement(element, parent, corrupted, elementText, elementTagName,
+ attrs, isAttr);
+ } on ArgumentError { // Thrown by _ThrowsNodeValidator
+ rethrow;
+ } catch(e) { // Unexpected exception sanitizing -> remove
+ _removeNode(element, parent);
+ window.console.warn('Removing corrupted element $elementText');
+ }
}
/// Having done basic sanity checking on the element, and computed the
@@ -40911,23 +40943,23 @@ class _ValidatingTreeSanitizer implements NodeTreeSanitizer {
void _sanitizeElement(Element element, Node parent, bool corrupted,
String text, String tag, Map attrs, String isAttr) {
if (false != corrupted) {
+ _removeNode(element, parent);
window.console.warn(
'Removing element due to corrupted attributes on <$text>');
- _removeNode(element, parent);
return;
}
if (!validator.allowsElement(element)) {
- window.console.warn(
- 'Removing disallowed element <$tag>');
_removeNode(element, parent);
+ window.console.warn(
+ 'Removing disallowed element <$tag> from $parent');
return;
}
if (isAttr != null) {
if (!validator.allowsAttribute(element, 'is', isAttr)) {
+ _removeNode(element, parent);
window.console.warn('Removing disallowed type extension '
'<$tag is="$isAttr">');
- _removeNode(element, parent);
return;
}
}
« no previous file with comments | « no previous file | sdk/lib/html/dartium/html_dartium.dart » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698