406036 - putImageData draws random memory

Status: VERIFIED FIXED

(Core :: Graphics: Canvas2D, defect, P3)

Description

[Philip Taylor]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.1.11pre) Gecko/20071127 BonEcho/2.0.0.11pre
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.1.11pre) Gecko/20071127 BonEcho/2.0.0.11pre

In FF2 on Linux, some uses of putImageData, e.g.:
  ctx.putImageData(ctx.getImageData(0, 0, 128, 64), 0, 128);
on a 256x256 canvas, fill the destination with bits of memory instead of with the correct pattern.

(The soon-to-be-attached test case works for me in trunk builds on Linux. I haven't tested any other platform.)

Reproducible: Always

Comment 1

[Philip Taylor]

Attachment: demonstration

Comment 2

[:Gavin Sharp [email: [e-mail]]]

Confirmed on Mac branch (2.0.0.10), works on Trunk.

Comment 3

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Is this a regression relative to earlier branch builds, or has it always been present on branch?

Comment 4

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Pretty sure it's always been present on branch -- I'm looking at the code now, and it's a buffer overrun; the wrong index was used in a loop involving memcpy().  This might raise the sg:low rating.  I should have a patch shortly.

Comment 5

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Attachment: Untested patch

Here's an untested patch, based on visual inspection of the code and the crash site.  I've got a build going now.

Comment 6

[Daniel Veditz [:dveditz]]

I haven't been able to reproduce this on Windows but the code looks cross-platform. Would that point the finger at all the cairo_ calls in the else section rather than the loop in the if() you changed?

In a debugger I don't hit the loop you changed on the attached testcase, I hit the else section. If your patch is correct then you've stumbled on an unrelated potential overflow -- can we get a testcase for that too?

Comment 7

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Yeah, I've had problems on the mac debugging -- I can't get gdb to find debug info for a debug build!  Really annoying.

Comment 8

[Mike Schroepfer]

based on comment #4 moving to blocking list

Comment 9

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Attachment: patch

Ah ha, so here's the fix.

The problem was twofold:
1) The wrong loop end condition was being used, which I caught earlier (y instead of h)
2) imgPtr was being used as the start of the source image data, but that pointer had been iterated over previously and now pointed to the end of the data!

This fixes things for me.  I'm pretty sure I never caught this originally because I was writing this code on windows, in which case we hit the else path; the path that doesn't use memcpy doesn't have this problem.

Comment 10

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Comment on attachment 297093 [details] [diff] [review]
patch


>+typedef unsigned int cairo_font_type_t;
>+

Erm, ignore this part of this patch, needed this due to the version of cairo on my test system.

Comment 11

[Daniel Veditz [:dveditz]]

Comment on attachment 297093 [details] [diff] [review]
patch

approved for 1.8.1.12, a=dveditz for release-drivers

Comment 12

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Checked in on trunk and 1.8.

Comment 13

[Al Billings [:abillings - ex-MoCo]]

Verified in branch in Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.8.1.12pre) Gecko/2008012208 BonEcho/2.0.0.12pre.

I don't see any trunk patch anywhere here. This is a branch only bug.

Comment 14

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

The patch was for branch and trunk.

Comment 15

[Al Billings [:abillings - ex-MoCo]]

Ah, since "Version" is marked "!.8 Branch", it wasn't clear that this was a trunk bug as well.

This needs to be verified in trunk then.

Comment 16

[Al Billings [:abillings - ex-MoCo]]

Verified for trunk with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9b3pre) Gecko/2008012204 Minefield/3.0b3pre.