Validate vector lengths in ImageHostMsg_DidDownloadImage

Validate vector lengths in ImageHostMsg_DidDownloadImage

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

[sky, 2014-01-10 18:14:31]

Should this be a CHECK ?

[jln (very slow on Chromium), 2014-01-10 23:01:06]

On 2014/01/10 18:14:31, sky wrote:
> Should this be a CHECK ?

We should not let compromised renderers DoS the browser too easily, so I would
prefer if this was not a CHECK.

However, I feel that the root issue here is that we should not have two vectors.
This looks like a bad pattern.

Could we have one vector of std::pair<> instead? Or semething else that
similarly makes such a mistake impossible.

[sky, 2014-01-10 23:25:43]

Good point. A vector of pairs should be possible.

On Fri, Jan 10, 2014 at 3:01 PM,  <jln@chromium.org> wrote:
> On 2014/01/10 18:14:31, sky wrote:
>>
>> Should this be a CHECK ?
>
>
> We should not let compromised renderers DoS the browser too easily, so I
> would
> prefer if this was not a CHECK.
>
> However, I feel that the root issue here is that we should not have two
> vectors.
> This looks like a bad pattern.
>
> Could we have one vector of std::pair<> instead? Or semething else that
> similarly makes such a mistake impossible.
>
> https://codereview.chromium.org/133123006/

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Tom Sepez, 2014-01-13 18:55:48]

Generally, what you want to do here is call BadMessageReceived() to kill the
renderer, since it's already off the rails if its sending these kinds of
messages. Then the browser continues on its merry way.

[ianbeer, 2014-01-17 15:57:37]

Updated to kill the renderer on a bad message.

The proper fix, using a vector of pairs rather than two vectors, is a much
bigger CL since I also have to update all the consumers of those two vectors in
the browser.

I think it's better to get this fix in ASAP, then make the better fix later.

[sky, 2014-01-17 16:34:13]

LGTM

[commit-bot: I haz the power, 2014-01-17 16:48:42]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ianbeer@chromium.org/133123006/130001

[commit-bot: I haz the power, 2014-01-17 21:10:19]

Change committed as 245618

[ianbeer, 2014-01-17 23:21:01]

A revert of this CL has been created in
https://codereview.chromium.org/131493009/ by ianbeer@chromium.org.

The reason for reverting is: Discussion required on whether to call
ReceivedBadMessage to kill the renderer or not.