Fix potential XSS on the NTP

chrome/browser/resources/local_ntp/most_visited_single.js

Index: chrome/browser/resources/local_ntp/most_visited_single.js
diff --git a/chrome/browser/resources/local_ntp/most_visited_single.js b/chrome/browser/resources/local_ntp/most_visited_single.js
index d6a1d5ccf564cc9959d479188b76682839f3de97..b624a7eaa77dfef10f4742ff3412fffa0dc6d41f 100644
--- a/chrome/browser/resources/local_ntp/most_visited_single.js
+++ b/chrome/browser/resources/local_ntp/most_visited_single.js
@@ -271,15 +271,7 @@ var showTiles = function() {
   tiles = document.createElement('div');
 
   if (impressionUrl) {
-    if (navigator.sendBeacon) {

[Marc Treib, 2016/03/09 15:24:02]

navigator.sendBeacon has existed since Chrome 39, I don't think we need the
fallback anymore.

-      navigator.sendBeacon(impressionUrl);
-    } else {
-      // if sendBeacon is not enabled, we fallback to "a ping".
-      var a = document.createElement('a');
-      a.href = '#';
-      a.ping = impressionUrl;
-      a.click();
-    }
+    navigator.sendBeacon(impressionUrl);
     impressionUrl = null;
   }
 };
@@ -343,31 +335,25 @@ var renderTile = function(data) {
 
   tile.className = 'mv-tile';
   tile.setAttribute('data-tid', data.tid);
-  var tooltip = queryArgs['removeTooltip'] || '';
   var html = [];
   if (!USE_ICONS) {
     html.push('<div class="mv-favicon"></div>');
   }
   html.push('<div class="mv-title"></div><div class="mv-thumb"></div>');
-  html.push('<div title="' + tooltip + '" class="mv-x"></div>');
+  html.push('<div class="mv-x"></div>');
   tile.innerHTML = html.join('');
+  tile.lastElementChild.title = queryArgs['removeTooltip'] || '';
 
-  tile.href = data.url;
+  if (!data.url.startsWith('javascript:')) {

[jochen (gone - plz use gerrit), 2016/03/09 15:26:41]

what about blob URLs etc? Would prefer a whitelist approach over a blacklist

[Marc Treib, 2016/03/09 16:28:34]

I agree that a whitelist is generally nicer, but I don't know where we could get
an appropriate whitelist from - right now, TopSites supports "any" scheme,
though I hope to change that soon (context on crbug.com/584461). Are you okay
with a blacklist for now, and revisiting once we can actually produce a
meaningful whitelist?

+    tile.href = data.url;
+  }
   tile.title = data.title;
   if (data.impressionUrl) {
     impressionUrl = data.impressionUrl;
   }
   if (data.pingUrl) {
     tile.addEventListener('click', function(ev) {
-      if (navigator.sendBeacon) {
-        navigator.sendBeacon(data.pingUrl);
-      } else {
-        // if sendBeacon is not enabled, we fallback to "a ping".
-        var a = document.createElement('a');
-        a.href = '#';
-        a.ping = data.pingUrl;
-        a.click();
-      }
+      navigator.sendBeacon(data.pingUrl);
     });
   }
   // For local suggestions, we use navigateContentWindow instead of the default