Last Comment Bug 759891 - Update libjpeg-turbo to 1.2.x branch r831 (or later)
: Update libjpeg-turbo to 1.2.x branch r831 (or later)
Status: RESOLVED FIXED
[sg:dupe 759802][advisory-tracking+]
: sec-other
Product: Core
Classification: Components
Component: ImageLib (show other bugs)
: Trunk
: All All
: -- normal with 1 vote (vote)
: mozilla16
Assigned To: Justin Lebar (not reading bugmail)
:
: Milan Sreckovic [:milan]
Mentors:
Depends on: 698519
Blocks: CVE-2012-2806
  Show dependency treegraph
 
Reported: 2012-05-30 13:58 PDT by Justin Lebar (not reading bugmail)
Modified: 2012-07-20 18:39 PDT (History)
11 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
unaffected
fixed
fixed
fixed
unaffected


Attachments
Part 1: Update MOZCHANGES file. (967 bytes, patch)
2012-06-04 22:43 PDT, Justin Lebar (not reading bugmail)
jmuizelaar: review+
Details | Diff | Splinter Review
Part 2: Update the code. (26.51 KB, patch)
2012-06-04 22:44 PDT, Justin Lebar (not reading bugmail)
jmuizelaar: review+
akeybl: approval‑mozilla‑aurora-
akeybl: approval‑mozilla‑beta-
akeybl: approval‑mozilla‑esr10-
Details | Diff | Splinter Review

Description Justin Lebar (not reading bugmail) 2012-05-30 13:58:08 PDT
libjpeg-turbo 1.2.x r831 fixes a potential security vulnerability.  We should update ASAP.

Ryan, are you interested in doing this again?
Comment 1 Ryan VanderMeulen [:RyanVM] 2012-05-31 03:52:23 PDT
I'm traveling until the end of next week. I can do it if nobody else does first.
Comment 2 Justin Lebar (not reading bugmail) 2012-06-04 22:43:36 PDT
Created attachment 630079 [details] [diff] [review]
Part 1: Update MOZCHANGES file.
Comment 3 Justin Lebar (not reading bugmail) 2012-06-04 22:44:03 PDT
Created attachment 630080 [details] [diff] [review]
Part 2: Update the code.

I'll probably fold these two csets together when I check them in, but I thought it would be easier to review separately.
Comment 4 Justin Lebar (not reading bugmail) 2012-06-04 22:48:24 PDT
The process I used to generate part 2 was:

 * Update my svn clone to r831
 * diff -r -U8 media/libjpeg ~/my/libjpeg-turbo/clone > ~/patch
 * Clean up ~/patch (remove spurious differences, e.g. because libjpeg-turbo has a different Makefile.in)
Comment 5 Justin Lebar (not reading bugmail) 2012-06-04 22:53:12 PDT
https://tbpl.mozilla.org/?tree=Try&rev=26474b1d7fee
Comment 6 Justin Lebar (not reading bugmail) 2012-06-05 08:06:46 PDT
Looks good on try; I'll land once I get r+ on the other patch.
Comment 7 Jeff Muizelaar [:jrmuizel] 2012-06-05 08:24:34 PDT
Comment on attachment 630080 [details] [diff] [review]
Part 2: Update the code.

Review of attachment 630080 [details] [diff] [review]:
-----------------------------------------------------------------

Sure
Comment 8 Justin Lebar (not reading bugmail) 2012-06-05 08:28:57 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/d10a38139eb8
Comment 9 Justin Lebar (not reading bugmail) 2012-06-05 08:33:55 PDT
Comment on attachment 630080 [details] [diff] [review]
Part 2: Update the code.

[Approval Request Comment]
Security fix.  No string changes.

If we wanted a smaller patch, we could probably cherry-pick the fix from libjpeg-turbo, instead of upgrading wholesale.  But I think landing what we're landing in nightly onto branches makes sense, because that's what we're testing.

Happy to let this bake on trunk for a day or two, but nom'ing now so it's on everyone's radar.
Comment 10 Joe Drew (not getting mail) 2012-06-05 10:59:08 PDT
I would much rather have a targeted patch.
Comment 11 Justin Lebar (not reading bugmail) 2012-06-05 11:47:39 PDT
(In reply to Joe Drew (:JOEDREW!) from comment #10)
> I would much rather have a targeted patch.

My concern is just that we're going to test the full r831 on Nightly, so we'd be introducing an effectively new version of the jpeg decoder just for branches.

Maybe DRC could verify for us that cherry-picking the fix is safe.  I'll post a patch.
Comment 12 Justin Lebar (not reading bugmail) 2012-06-05 11:56:59 PDT
> I'll post a patch.

...to bug 759802.
Comment 13 DRC 2012-06-05 13:18:49 PDT
830->831 does not depend on any prior patches, so it should be safe to apply it without any of the other 1.2.1 patches.
Comment 14 Ed Morley [:emorley] 2012-06-06 08:45:41 PDT
https://hg.mozilla.org/mozilla-central/rev/d10a38139eb8
Comment 15 Daniel Veditz [:dveditz] 2012-06-06 10:42:59 PDT
Despite the approval requests here, do we want the smaller patch in bug 759802 instead? Definitely for ESR we'd want the small one, for Aurora we could go with this one, and Beta is a toss-up in my mind.
Comment 16 Justin Lebar (not reading bugmail) 2012-06-06 12:06:57 PDT
I think taking the smaller patch on Beta and ESR and the larger patch on Aurora would be fine, but I'd like the release drivers' input, since that effectively adds a second libjpeg upgrade, which adds some risk.
Comment 17 Alex Keybl [:akeybl] 2012-06-06 16:15:42 PDT
Comment on attachment 630080 [details] [diff] [review]
Part 2: Update the code.

Let's just take the patch in bug 759802 for all branches, and wait till FF16 to take the rest of the updates to libjpeg since there doesn't appear to be any significant user benefit at this time.

Note You need to log in before you can comment on or make changes to this bug.