Is there a reason to suspect SkImageSource/SkCodecImageGenerator/Skia codecs in general are insecure?  If this scenario cannot occur in practice, is there a way to assert  instead?

senorblanco@/reed@ are prolly more familiar with the cross-process embedded picture serialization ban (IIRC it had to do with our confidence in SkPicture fuzzing coverage), but I'm not sure the same motivation is applicable to SkImage(Source).

I think in general, we don't do any image decoding in a trusted process.

I would like to really understand this situation. From a naive perspective, codecs are just code that we can control/test/fuzz/fix just like Skia itself (and command-buffer and all of the new deserialization code being written). Why do we think decoding is so risky that we cannot use it -vs- the risk of not making our other code robust?

On a different front, I was under the impression that we were exploring worker processes to perform decoding into shared memory. Is that still on the table?

As for why we disallow decoding in a privileged process, I think palmer@ would be able to answer that better.

We do have utility processes in Chrome used for image decoding in general. But did you mean worker processes in the context of out-of-process raster? The plan there is to pre-decode everything in the renderer and only serialize decodes, similar to how cc does most pre-decoding today. There are a few cases where images are still buried deep inside skia data structures, which we are working on making visible in cc.

My point here was that within the current system, we have opaque SkImageFilters on CompositorFrames that are serialized cross-process, and there are no security checks to ensure that we are not serializing images in these filters to the browser and performing the decode there. Since the serialization is going through skia, there should at least be a a way to assert that it does not include encoded images.

I don't think I understand all the moving parts in play here. It sounds like khushalsagar wants a static assertion that we won't run (a) certain decoder(s) in a privileged process, and that we already have such assertions for some?

We disable serialization of SkPictures in the renderer (https://cs.chromium.org/chromium/src/third_party/skia/src/effects/SkPictureImageFilter.cpp?l=85), but we don't have a similar limitation for SkImages (https://cs.chromium.org/chromium/src/third_party/skia/src/effects/SkImageSource.cpp?l=78). Which means its possible to have an encoded image serialized to the browser, and eventually decoded there using an SkCodec.

#7: I see. Yeah, it would definitely be good to have a guarantee against that happening.

As a general security principle, we don't want to deserialize any inputs from a complex grammar in any privileged process.

Skia has a SkPixelSerializer callback class that can be passed to methods like SkPicture::serialize and SkImage::encodeToData. Chrome/blink can pass in a subclass of this that will allow them to restrict any image formats they wish (e.g. encoded). It is fine-grained, meaning it can selectively allow/restrict some formats and not others, and it can decide to either abort a given image's serialization, or transform it into a different form.

Patch up for review: https://chromium-review.googlesource.com/c/chromium/src/+/688656.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/116159dfe170dcc2cf6f3b2a04a292130f6b79e2

commit 116159dfe170dcc2cf6f3b2a04a292130f6b79e2
Author: Khushal <khushalsagar@chromium.org>
Date: Thu Oct 05 01:54:51 2017

cc/ipc: Disallow codec backed SkImageFilters on CompositorFrames.

CompositorFrames can have opaque SkImageFilters which internally may
contain codec backed images. No cc client should be using such filters.
Assert this during filter serialization.

R=enne@chromium.org, reed@chromium.org

Bug:  768511 
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I222cf4f7189e879537c19d723bf57b739aa9ad57
Reviewed-on: https://chromium-review.googlesource.com/688656
Commit-Queue: Khushal <khushalsagar@chromium.org>
Reviewed-by: Florin Malita <fmalita@chromium.org>
Reviewed-by: Mike Reed <reed@chromium.org>
Reviewed-by: Chris Palmer <palmer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#506619}
[modify] https://crrev.com/116159dfe170dcc2cf6f3b2a04a292130f6b79e2/cc/ipc/cc_param_traits.cc
[modify] https://crrev.com/116159dfe170dcc2cf6f3b2a04a292130f6b79e2/cc/ipc/cc_param_traits.h
[modify] https://crrev.com/116159dfe170dcc2cf6f3b2a04a292130f6b79e2/skia/BUILD.gn
[modify] https://crrev.com/116159dfe170dcc2cf6f3b2a04a292130f6b79e2/skia/ext/skia_utils_base.cc
[modify] https://crrev.com/116159dfe170dcc2cf6f3b2a04a292130f6b79e2/skia/ext/skia_utils_base.h
[add] https://crrev.com/116159dfe170dcc2cf6f3b2a04a292130f6b79e2/skia/ext/skia_utils_base_unittest.cc
[modify] https://crrev.com/116159dfe170dcc2cf6f3b2a04a292130f6b79e2/skia/public/interfaces/image_filter_struct_traits.h