Support a different source for fillMostVisited

Support a different source for fillMostVisited

In certain cases, generates a suggestion object from the iframe params.

BUG=None

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=213278

[Mathieu, 2013-07-19 19:36:58]

Hi Samarth, 

This is to support my suggestions in the iframe. PTAL.

[samarth, 2013-07-19 22:16:39]

https://codereview.chromium.org/19803006/diff/1/chrome/browser/resources/loca...
File chrome/browser/resources/local_ntp/most_visited_util.js (right):

https://codereview.chromium.org/19803006/diff/1/chrome/browser/resources/loca...
chrome/browser/resources/local_ntp/most_visited_util.js:114: // Means that we
get suggestion data from the server. Create data object.
These fields are currently set by C++ code in chrome and now you're allowing the
server to set them. I suspect you may need to some extra sanity checks/escaping
to make sure we're not introducing an attack vector. (Remind me how exactly are
these fields used by the downstream JS).

[Mathieu, 2013-07-22 17:32:50]

Thanks!

https://codereview.chromium.org/19803006/diff/1/chrome/browser/resources/loca...
File chrome/browser/resources/local_ntp/most_visited_util.js (right):

https://codereview.chromium.org/19803006/diff/1/chrome/browser/resources/loca...
chrome/browser/resources/local_ntp/most_visited_util.js:114: // Means that we
get suggestion data from the server. Create data object.
On 2013/07/19 22:16:40, samarth wrote:
> These fields are currently set by C++ code in chrome and now you're allowing
the
> server to set them. I suspect you may need to some extra sanity
checks/escaping
> to make sure we're not introducing an attack vector. (Remind me how exactly
are
> these fields used by the downstream JS).

The downstream code will create title and thumbnail elements inside the iframe
using the fields that we set here in |data| (see
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/res...
for the thumbnail). Notably this will create an image element with src
data.thumbnailUrl, so I've checked it for javascript below, if that is possibly
a vector.

[Mathieu, 2013-07-23 16:43:20]

On 2013/07/22 17:32:50, Mathieu Perreault wrote:
> Thanks!
> 
>
https://codereview.chromium.org/19803006/diff/1/chrome/browser/resources/loca...
> File chrome/browser/resources/local_ntp/most_visited_util.js (right):
> 
>
https://codereview.chromium.org/19803006/diff/1/chrome/browser/resources/loca...
> chrome/browser/resources/local_ntp/most_visited_util.js:114: // Means that we
> get suggestion data from the server. Create data object.
> On 2013/07/19 22:16:40, samarth wrote:
> > These fields are currently set by C++ code in chrome and now you're allowing
> the
> > server to set them. I suspect you may need to some extra sanity
> checks/escaping
> > to make sure we're not introducing an attack vector. (Remind me how exactly
> are
> > these fields used by the downstream JS).
> 
> The downstream code will create title and thumbnail elements inside the iframe
> using the fields that we set here in |data| (see
>
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/res...
> for the thumbnail). Notably this will create an image element with src
> data.thumbnailUrl, so I've checked it for javascript below, if that is
possibly
> a vector.

Hi Samarth, please take a look, the server code went in and I'd love to push
this forward.

[samarth, 2013-07-23 18:27:17]

lgtm

[commit-bot: I haz the power, 2013-07-23 19:09:18]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mathp@chromium.org/19803006/5001

[commit-bot: I haz the power, 2013-07-23 23:53:56]

Change committed as 213278