Skip to content
This repository has been archived by the owner on Aug 1, 2024. It is now read-only.

Commit

Permalink
Automated g4 rollback of changelist 214621663.
Browse files Browse the repository at this point in the history
*** Reason for rollback ***

This introduced an XSS in GWS. b/125799080

*** Original change description ***

Fix(safedomtreeprocessor): closing empty element using XML style /> not valid on IE cause by IE's XMLSerializer

- use innerHTML instead of XMLSerializer to get string version of sanitized HTML tree
- added test for this issue
- updated affected test http://sponge/54fb1dcf-8d59-4b42-9faf-9702b24466c1

demo: http://pmelendez.pit.corp.google.com:8888/search?q=nintendo+switch&e=4197585

RELNOTES: Fix safedomtreeprocessor.processToString closing empty element using /> on IE.

***

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=235204275
  • Loading branch information
sirdarckcat authored and nreid260 committed Feb 23, 2019
1 parent a2daa5f commit c79ab48
Show file tree
Hide file tree
Showing 3 changed files with 8 additions and 16 deletions.
4 changes: 2 additions & 2 deletions closure/goog/html/sanitizer/htmlsanitizer_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1648,11 +1648,11 @@ function testUrlWithCredentials() {


function testClobberedForm() {
var input = '<form><input name="nodeType"></form>';
var input = '<form><input name="nodeType" /></form>';
// Passing a string in assertSanitizedHtml uses assertHtmlMatches, which is
// also vulnerable to clobbering. We use a regexp to fall back to simple
// string matching.
var expected = new RegExp('<form><input name="nodeType"></form>');
var expected = new RegExp('<form><input name="nodeType" /></form>');
assertSanitizedHtml(
input, expected,
new goog.html.sanitizer.HtmlSanitizer.Builder()
Expand Down
9 changes: 6 additions & 3 deletions closure/goog/html/sanitizer/safedomtreeprocessor.js
Original file line number Diff line number Diff line change
Expand Up @@ -108,9 +108,12 @@ SafeDomTreeProcessor.prototype.processToString = function(html) {
newRoot.appendChild(newTree);
newTree = newRoot;
}

// Serialized string of the sanitized DOM without root span tag
return newTree.innerHTML;
// The XMLSerializer will add a spurious xmlns attribute to the root node.
var serializedNewTree = new XMLSerializer().serializeToString(newTree);
// Remove the outer span before returning the string representation of the
// processed copy.
return serializedNewTree.slice(
serializedNewTree.indexOf('>') + 1, serializedNewTree.lastIndexOf('</'));
};

/**
Expand Down
11 changes: 0 additions & 11 deletions closure/goog/html/sanitizer/safedomtreeprocessor_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -73,17 +73,6 @@ testSuite({
input, new NoopProcessor().processToString(input));
},

testEmptyTag() {
var input = '<div></div>';
var actual = new NoopProcessor().processToString(input);

if (SafeDomTreeProcessor.SAFE_PARSING_SUPPORTED) {
assertEquals(input, actual);
} else {
assertEquals('', actual);
}
},

testTagChanged() {
var processor = new NoopProcessor();
processor.createElementWithoutAttributes = anchorToFoo;
Expand Down

7 comments on commit c79ab48

@leonklingele
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

πŸ™€

@jlagarespo
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

πŸ‘€

@kinduff
Copy link

@kinduff kinduff commented on c79ab48 Apr 3, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

πŸ˜†

@bciccarelli
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jlagarespo
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AluminumChassis same lol

@patrickbard
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Haha I came from https://www.youtube.com/watch?v=lG7U3fuNw3A

samething here

@Spangonagornomb
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The DEA THE GOOGLE HAS XSS
well done

Please sign in to comment.