Bugzilla

After looking at the patch/testcase that Christian supplied me, this seems potentially a pretty insidious bug in terms of the potential failures this could have caused just about anywhere else in the code.

GfxColorTypeToSkiaColorType can return a Skia SkColorType whose bytes-per-pixel is larger than the Moz2D SurfaceFormat. This can then cause Skia to easily overrun via read or write the supplied buffer by multiples of the actual data's size.

This is insidious precisely because I can't predict all the potential entry-points this could impact, but it can easily be avoided by just fixing it at the source of the problem. In this case, I make GfxColorTypeToSkiaColorType return a SkColorType value whose size is always less than other unmappable SurfaceFormats, i.e. kAlpha_8_SkColorType, whose size is always 1.

The part of the fix that gets tricky is also an issue, that if we allocate pixel data inside Skia based on that SkColorType, we need to make sure we don't call into Moz2D rasterization routines that assume the corresponding SurfaceFormat has a larger bytes-per-pixel or stride. Therefor, I insert checks to assure these values always agree for SourceSurfaceSkia and DrawTargetSkia on construction, which should prevent us from getting caught by this.

I'd almost rank this as higher than sec-high because I can't quite predict all the vulnerabilities this could have caused.

After looking at the patch/testcase that Christian supplied me, this seems potentially a pretty insidious bug in terms of the potential failures this could have caused just about anywhere else in the code.

GfxColorTypeToSkiaColorType can return a Skia SkColorType whose bytes-per-pixel is larger than the Moz2D SurfaceFormat. This can then cause Skia to easily overrun via read or write the supplied buffer by multiples of the actual data's size.

This is insidious precisely because I can't predict all the potential entry-points this could impact, but it can easily be avoided by just fixing it at the source of the problem. In this case, I make GfxColorTypeToSkiaColorType return a SkColorType value whose size is always less than other unmappable SurfaceFormats, i.e. kAlpha_8_SkColorType, whose size is always 1.

The part of the fix that gets tricky is also an issue, that if we allocate pixel data inside Skia based on that SkColorType, we need to make sure we don't call into Moz2D rasterization routines that assume the corresponding SurfaceFormat has a larger bytes-per-pixel or stride. Therefor, I insert checks to assure these values always agree for SourceSurfaceSkia and DrawTargetSkia on construction, which should prevent us from getting caught by this.