891693 - (CVE-2013-6629) JPEG info leak

Status: RESOLVED FIXED

(Core :: Graphics: ImageLib, defect)

Description

[Michal Zalewski]

This page contains an alternating pattern of kitty.jpg (a legit hello kitty image) and 55.jpg, an image that is 'decoded' to what looks like uninitialized memory:

http://lcamtuf.coredump.cx/jpeg_leak/

In most browsers, reloading this page a couple of times eventually causes a distorted version of 'hello kitty' to appear in one of the 55.jpg squares. In Firefox, it's relatively difficult to hit that condition, but it still happens every now and then.

At the bare minimum, this + <canvas> seems like a way to steal images across domains, although I suspect that the memory may contain non-image stuff, too.

Safari & others are also affected.

Comment 1

[Christoph Diehl [:posidron]]

Adding some GFX people.

Comment 2

[Milan Sreckovic [:milan] (needinfo for best results)]

On OSX 10.8, happens in Safari on the initial load.

Comment 3

[Michal Zalewski]

FWIW, an earlier unitialized memory bug in BMP handling allowed gynvael@ to steal cookies in Firefox:

http://vexillium.org/?sec-ff

Comment 4

[Milan Sreckovic [:milan] (needinfo for best results)]

We trigger the "bad Huffman code" warning in jdhuff.c; I assume it's related:

/* With garbage input we may reach the sentinel value l = 17. */
  if (l > 16) {
    WARNMS(state->cinfo, JWRN_HUFF_BAD_CODE);
    return 0;			/* fake a zero as the safest result */
  }

Comment 5

[Joe Drew (not getting mail)]

DRC, this will be of interest to you.

Comment 6

[Michal Zalewski]

I think it's probbly not jdhuff.c; the file contains a SOS record for component 2 or 3, but not for component 1. A closer look at get_sos() in jdmarker.c is probably a better starting point.

(Just started looking into it, take this with a grain of salt.)

Comment 7

[Michal Zalewski]

Yeah, I think it's that. We have Cr and Cb (chroma), but not Y (luma). The library doesn't notice that and applies chroma info over uninitialized memory?

Comment 8

[Michal Zalewski]

Large repro:

http://lcamtuf.coredump.cx/jpeg_leak/random.jpg

Comment 9

[DRC]

If it's happening in Safari, then it sounds like an issue that is endemic to libjpeg in general, not just -turbo. Can someone confirm that?

Comment 10

[Michal Zalewski]

Yup, it affects jpeg6b and derivatives.

Comment 11

[DRC]

hmmm...  So does that mean it doesn't affect jpeg-8?  Trying to get a handle on why such a bug escaped everyone's attention for the last 15 years.

Comment 12

[Michal Zalewski]

I see no indication that it's intentionally fixed, but there are some differences in the code that make it a bit hard to tell. 

They do accept a slightly modified version (http://lcamtuf.coredump.cx/jpeg_leak/55-jpeg9-variant.jpg), but I didn't get any non-deterministic behavior. Didn't put a lot of effort into it because the use of jpeg8/jpeg9 seems to be minimal.

As for the reasons, I suspect that everybody is fuzzing for crashes and working from that. And IJG jpeg doesn't crash ;p

/mz

Comment 13

[Michal Zalewski]

I'm afraid I might have another one that may be separate from the IJG issue above. It has a different SOS and it only seems to decode non-deterministically in Chrome, Firefox, and with djpeg from libjpeg-turbo when malloc() is tweaked to randomly memset the returned buffers:

http://lcamtuf.coredump.cx/jpeg_leak/182.jpg

Comment 14

[Michal Zalewski]

I'll be OOO for three weeks, so just a quick update:

1) Chrome folks have a tentative fix that bans SOS markers with repeated component IDs. I am not 100% sure this is bulletproof - I'm not sure if there are earlier checks int he code that would prevent, for example, a two-component JPEG with non-repeated components 2 and 3, but not 1. Perhaps?

2) In addition to the SOS issue that appears to be shared by jpeg6b and libjpeg-turbo, there is also a separate issue in turbo that appears to be related to the DHT marker:

http://lcamtuf.coredump.cx/jpeg_leak/turbo-dht.jpg

Here's the non-error-causing parent from which it's derived:

http://lcamtuf.coredump.cx/jpeg_leak/turbo-dht-orig.jpg

Shift-reload a couple of times in Firefox to see the problem.

Comment 15

[David Bolter [:davidb] (NeedInfo me for attention)]

Milan is this bad? We are wondering if sec-high is too conservative.

Comment 16

[Milan Sreckovic [:milan] (needinfo for best results)]

Comment 2 could be arguing that it could be higher, but I'm not sure it's the same kind of "uninitialized memory" issue.  I think we'll just display garbage, not sure the attack could control where the garbage comes from...

Comment 17

[David Bolter [:davidb] (NeedInfo me for attention)]

Thanks. Note by 'conservative' we meant it might be rated too highly and were similarly thinking that displaying garbage is not too harmful on its own.

Comment 18

[Michal Zalewski]

The PoC allows cross-domain image theft.

Unless demonstrated otherwise, I would caution against assuming that it's just "garbage": images can be large, and browsers can be made to use and free lots of memory for very specific things, which often makes the distinction between "read arbitrary address" and "read a specific address" fairly insignificant.

Comment 19

[Daniel Veditz [:dveditz]]

Milan: please find an owner for this bug. We could probably just adopt the fix chrome has.

Comment 20

[Milan Sreckovic [:milan] (needinfo for best results)]

(In reply to Daniel Veditz [:dveditz] from comment #19)
> Milan: please find an owner for this bug. We could probably just adopt the
> fix chrome has.

Michal, you mentioned "Chrome folks have a tentative fix" in Comment 14 - can you point us to it/them?

Comment 21

[Michal Zalewski]

 1 diff --git a/jdmarker.c b/jdmarker.c
 2 index 0d5da67..efc68c7 100644
 3 --- a/jdmarker.c
 4 +++ b/jdmarker.c
 5 @@ -303,7 +303,7 @@ get_sos (j_decompress_ptr cinfo)
 6  /* Process a SOS marker */
 7  {
 8    INT32 length;
 9 -  int i, ci, n, c, cc;
10 +  int i, ci, n, c, cc, pi;
11    jpeg_component_info * compptr;
12    INPUT_VARS(cinfo);
13
14 @@ -347,6 +347,12 @@ get_sos (j_decompress_ptr cinfo)
15
16      TRACEMS3(cinfo, 1, JTRC_SOS_COMPONENT, cc,
17              compptr->dc_tbl_no, compptr->ac_tbl_no);
18 +
19 +    /* This CSi (cc) should differ from the previous CSi */
20 +    for (pi = 0; pi < i; pi++) {
21 +      if (cinfo->cur_comp_info[pi] == compptr)
22 +        ERREXIT1(cinfo, JERR_BAD_COMPONENT_ID, cc);
23 +    }
24    }

Note that this doesn't cover the DHT issue, just the SOS one.

Comment 22

[Milan Sreckovic [:milan] (needinfo for best results)]

Attachment: Ban SOS with repeated components IDs

This is a patch from comment 21.  It does fix the SOS problem for me.  Unsure how to proceed and who to have review this and if there are any upstream implications.  Jeff?

Comment 23

[DRC]

I don't really understand the ramifications of this fully and haven't had enough time to dig into it, but since you're mentioning jdmarker.c, one difference in libjpeg-turbo relative to jpeg-6b is that libjpeg-turbo handles JPEG images with duplicate component IDs.  Specifically, some commercial software generates CMYK/YCCK JPEG images in which the K component has the same ID as the first component (C or Y.)  Such images are out of spec, but apparently Adobe Reader handles them without error, and SumatraPDF requested that we do likewise.  The specific patch relative to jpeg-6b is as follows:


***************
*** 323,333 ****
    /* Collect the component-spec parameters */
  
    for (i = 0; i < n; i++) {
      INPUT_BYTE(cinfo, cc, return FALSE);
      INPUT_BYTE(cinfo, c, return FALSE);
      
!     for (ci = 0, compptr = cinfo->comp_info; ci < cinfo->num_components;
  	 ci++, compptr++) {
!       if (cc == compptr->component_id)
  	goto id_found;
      }
--- 325,339 ----
    /* Collect the component-spec parameters */
  
+   for (i = 0; i < MAX_COMPS_IN_SCAN; i++)
+     cinfo->cur_comp_info[i] = NULL;
+ 
    for (i = 0; i < n; i++) {
      INPUT_BYTE(cinfo, cc, return FALSE);
      INPUT_BYTE(cinfo, c, return FALSE);
      
!     for (ci = 0, compptr = cinfo->comp_info;
! 	 ci < cinfo->num_components && ci < MAX_COMPS_IN_SCAN;
  	 ci++, compptr++) {
!       if (cc == compptr->component_id && !cinfo->cur_comp_info[ci])
  	goto id_found;
      }


I don't think that the patch you're proposing affects the above, but I'd love it if someone could confirm that for me.

Comment 24

[Michal Zalewski]

Hey,

We're getting close to three months on this bug; I'd like to tentatively aim for releasing the test cases and the tool used to generate them somewhere around October 4.

Comment 25

[Michal Zalewski]

Hey all,

A simple fix for the DHT issue appears to be to make sure that huffval in jdmarker.c gets zeroed before use:

--- a/jdmarker.c
+++ b/jdmarker.c
@@ -464,6 +464,8 @@ get_dht (j_decompress_ptr cinfo)
     for (i = 0; i < count; i++)
       INPUT_BYTE(cinfo, huffval[i], return FALSE);
 
+    MEMZERO(&huffval[count], (256 - count) * SIZEOF(UINT8));
+
     length -= count;
 
     if (index & 0x10) {                /* AC table definition */

Hope this helps. I'd like to publish the test cases and the tool used to generate them, so if there is a desire to ship libjpeg-turbo and / or Firefox with the fixes ahead of the time, it would be good to make it happen very soon :-)

Comment 26

[Milan Sreckovic [:milan] (needinfo for best results)]

(In reply to Michal Zalewski from comment #25)
> Hey all,
> 
> A simple fix for the DHT issue appears to be to make sure that huffval in
> jdmarker.c gets zeroed before use:
> ...

Unfortunately, this doesn't seem to fix the problem for me (OS X 10.8.4), I can still see the corruption in the browser.  Your suggestion from Comment 21 does fix the problem for me, seemingly with or without the zeroing.

Comment 27

[Michal Zalewski]

To be clear, there are two related info leak bugs. One is fixed by comment 21 (http://lcamtuf.coredump.cx/jpeg_leak/), the other should be fixed by that last one (http://lcamtuf.coredump.cx/jpeg_leak/turbo-dht.jpg).

Comment 28

[Milan Sreckovic [:milan] (needinfo for best results)]

That clears it up, thanks.

Comment 29

[Michal Zalewski]

FYI, I will release proof-of-concept images and the associated fuzzer on Oct 31. It's probably good for Firefox to make progress toward shipping the fixes around that time.

DRC: It would probably be good to include the fixes upstream, too, even if just to avoid confusion.

Comment 30

[DRC]

I absolutely agree.  I would ask that someone submit each fix separately into our SourceForge tracker along with a test case that demonstrates the issue outside of Mozilla.  If that isn't possible, then please explain further.  Honestly, I've been having trouble following much of this discussion thus far, so any explanation at this point is appreciated.

Comment 31

[DRC]

Comment on attachment 800490 [details] [diff] [review]
Ban  SOS with repeated components IDs

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

I don't see any issues with this, although I would love it if someone could explain exactly the mechanism by which this prevents a memory leak and why the existing code in libjpeg-turbo that handles duplicate component IDs doesn't also handle this.

Comment 32

[DRC]

Sorry to bug everyone about this, but it's been driving me nuts and is making it difficult for me to participate in this forum.  I can't seem to figure out how to make Bugzilla "accept" my reviews.  Despite having published a review on a couple of different bugs, it keeps e-mailing me and telling me I still need to review them.  If there's a way to do what I'm trying to do, it is very non-obvious.

Comment 33

[Dana Keeler (she/her) [:keeler]]

(In reply to DRC from comment #32)
> Sorry to bug everyone about this, but it's been driving me nuts and is
> making it difficult for me to participate in this forum.  I can't seem to
> figure out how to make Bugzilla "accept" my reviews.  Despite having
> published a review on a couple of different bugs, it keeps e-mailing me and
> telling me I still need to review them.  If there's a way to do what I'm
> trying to do, it is very non-obvious.

In the review view, there will be a list of flags below the navigation/comment/review area. It'll look like this:

milan: 	review 	?	[e-mail]
milan: 	review 	?	[e-mail]

For the review with your email on it, you should be able to change the "?" to " ", "-", or "+", depending on what you want to do ("clear", "review-", or "review+" in this case). Then hit "publish".

Comment 34

[DRC]

I see that view, but it does not allow me to modify the flags.  They are just static text.

Comment 35

[Daniel Veditz [:dveditz]]

(In reply to DRC from comment #34)
> I see that view, but it does not allow me to modify the flags.  They are
> just static text.

You didn't have "editbugs" permissions so I gave you that. I think without that large parts of bugzilla UI is turned off.

Comment 36

[Michal Zalewski]

Just for the record, after several delays, Chrome is going to release the fixes on Nov 12. I will probably post an advisory around that time.

Comment 37

[Milan Sreckovic [:milan] (needinfo for best results)]

Comment on attachment 800490 [details] [diff] [review]
Ban  SOS with repeated components IDs

[Security approval request comment]
How easily could an exploit be constructed based on the patch?
The PoC allows cross-domain image theft.


Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?
No.


Which older supported branches are affected by this flaw?
All.

If not all supported branches, which bug introduced the flaw?

Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?
This is a libjpeg change.  The patch is trivial to port, but anything in libjpeg effects a lot, so from that point of view, there is risk.

How likely is this patch to cause regressions; how much testing does it need?
Not likely, but if anything, it would be false positives, where an image is tagged as invalid, but it's actually valid.  Or, that the invalid images are used already:

"... some commercial software generates CMYK/YCCK JPEG images in which the K component has the same ID as the first component (C or Y.)  Such images are out of spec, but apparently Adobe Reader handles them without error, and SumatraPDF requested that we do likewise..."

Comment 38

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

Comment on attachment 800490 [details] [diff] [review]
Ban  SOS with repeated components IDs

sec-approval+ for trunk.

You should probably create and nominate Aurora, Beta, and ESR24 patches since this is a sec-high and a tiny patch.

Comment 39

[DRC]

OK, I really need an executive roll-up on this bug, because I'm having difficulty understanding how banning repeated component ID's solves the issue of handling JPEGs that have no SOS for the Y component.  Was not the original issue that libjpeg assumes that images always have a Y SOS?

Comment 40

[Michal Zalewski]

I think that there are other locations that enforce 1 or 3 components, and an earlier check that requires component IDs to be within that range.

So, AFAICT, BW images can only have a single component in SOS and the ID must be 1. RGB images need to have three components in SOS and the IDs must be some combination of 1, 2, and 3. 

Because of this, the only way to trigger the bug is to have duplicate components in an RGB image (e.g., 1-1-3, 2-3-3, etc) and backporting the duplicate-component check from jpeg9 is good enough, even though it's not an intuitive fix.


/mz

Comment 41

[Milan Sreckovic [:milan] (needinfo for best results)]

(In reply to Al Billings [:abillings] from comment #38)
> Comment on attachment 800490 [details] [diff] [review]
> Ban  SOS with repeated components IDs
> 
> sec-approval+ for trunk.

Checkin-needed for trunk only.  While it's a tiny patch, it is jpeg, so I would like it on trunk alone for a couple of days.

Comment 42

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/b2g-inbound/rev/158afa8bba37

Comment 43

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/158afa8bba37

Comment 44

[DRC]

Michal, can you comment on why you posted details of this bug to 

http://seclists.org/fulldisclosure/2013/Nov/83

??  I thought the idea was to keep this under wraps until Mozilla agreed on a fix, so as not to encourage people to exploit it?  Seems like posting it on an external site subverts the whole "secure bug" process.

I got blindsided by one of my users who stumbled upon the above post and was asking me why I hadn't checked in the fixes yet.  Well, because I was waiting for someone to post the "approved" Mozilla patches upstream or at least give me a high sign that they were final.  I assume that the repeated SOS issue is resolved, but I was never asked to review the DHT patch, so I don't know whether that issue was fully resolved yet or not.

Comment 45

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

Milan, please make branch patches now since this has been in for a few days and it appears the reporter just went public.

Comment 46

[Daniel Veditz [:dveditz]]

Chrome is using CVE-2013-6629 for this bug.

Comment 47

[Michal Zalewski]

DRC, in comment 24, I noted that I'm aiming to release the details on October 4. In comment 29, I moved this to October 31; and in comment 36, I finally proposed November 12. I had no feedback on any of the proposed deadlines, so I proceeded with the plan.

Comment 48

[DRC]

I am not a Mozilla developer, but if I were, I would have expected that you would have waited until the fixes were integrated into Mozilla and this bug was made public.  You can't just set your own arbitrary deadlines on a security patch.  The purpose of keeping this private is to not encourage exploitation of the issue before a fix is available to end users.

Has the DHT issue also been fully resolved?  It is unclear from reading the above comments.

Comment 49

[Michal Zalewski]

DRC, it's generally a two-way street, i.e., developers ask for a reasonable advance notice, and researchers expect a timely resolution of the underlying issue. There is no single rule, but in general, very few people in the OSS community support indefinite secrecy for security bugs.

Comment 50

[DRC]

I didn't say indefinite, but the idea of making it private in the first place is so that an end user patch is available before the details of the bug are disclosed.  That is, in case someone were to discover a way to exploit this, all a user would have to do to guard against the exploit is to update Firefox.

And you still are not answering my direct question regarding the DHT bug.

Comment 51

[Michal Zalewski]

Comment 21 provides a fix for the SOS bug. Comment 25 provides a fix for the DHT bug.

Comment 53

[Ryan VanderMeulen [:RyanVM]]

(In reply to Al Billings [:abillings] from comment #45)
> Milan, please make branch patches now since this has been in for a few days
> and it appears the reporter just went public.

The patch landed in comment 43 applies cleanly all the way down to b2g18. Just need the approvals to land.

Comment 54

[DRC]

You guys seem to be commenting only on the SOS patch but not the DHT patch.  Are both eventually going to be integrated into Mozilla?  Has the DHT patch been reviewed and regression tested?

Comment 55

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/d60b630fdd37
https://hg.mozilla.org/releases/mozilla-beta/rev/0297fa9b8006
https://hg.mozilla.org/releases/mozilla-esr24/rev/528cec574e53
https://hg.mozilla.org/releases/mozilla-b2g18/rev/c699a8e7bde9

needinfo? Milan for comment 54. Please reopen the bug and reset the status flags if more is needed here still.

Comment 56

[Milan Sreckovic [:milan] (needinfo for best results)]

Attachment: The DHT patch from comment 25

Comment 57

[DRC]

Comment on attachment 8336111 [details] [diff] [review]
The DHT patch from comment 25

The spacing was correct in comment 25.  Why did you change it?  Otherwise, I see no issues with this.

Comment 58

[DRC]

Comment on attachment 8336111 [details] [diff] [review]
The DHT patch from comment 25

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

Note also that I regression tested the performance and found it to be unchanged.

Comment 59

[Ryan VanderMeulen [:RyanVM]]

SOS patch:
https://hg.mozilla.org/releases/mozilla-b2g26_v1_2/rev/0297fa9b8006
https://hg.mozilla.org/releases/mozilla-b2g18_v1_1_0_hd/rev/c699a8e7bde9

Comment 60

[Milan Sreckovic [:milan] (needinfo for best results)]

Comment on attachment 8336111 [details] [diff] [review]
The DHT patch from comment 25

[Security approval request comment]
How easily could an exploit be constructed based on the patch?
Not easily.

Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?
The comment talks about "info leak", so it does identify that there is an issue.

Which older supported branches are affected by this flaw?
All
If not all supported branches, which bug introduced the flaw?
n/a

Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?
This is a libjpeg change.  The patch is trivial to port, but anything in libjpeg effects a lot, so from that point of view, there is risk.

How likely is this patch to cause regressions; how much testing does it need?
Not likely to cause a regression.

Comment 61

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

Comment on attachment 8336111 [details] [diff] [review]
The DHT patch from comment 25

sec-approval+ for trunk.

We should get this nominated and approved for branches where we want to take it.

Comment 62

[Lukas Blakk [:lsblakk] use ?needinfo]

please nom before Monday to get in our second-to-last Beta

Comment 63

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/364bd220cdb7

Will uplift to the release branches as soon as it's approved.

Comment 64

[Milan Sreckovic [:milan] (needinfo for best results)]

Comment on attachment 8336111 [details] [diff] [review]
The DHT patch from comment 25

The fix for the info leak came in two parts.  This is the second part, same reasoning for uplifting.

Comment 65

[Milan Sreckovic [:milan] (needinfo for best results)]

(In reply to [e-mail] [:lsblakk] from comment #62)
> please nom before Monday to get in our second-to-last Beta

Done very early on Monday, hopefully that counts...

Comment 66

[DRC]

Please note that the DHT patch needs to be reformatted for spacing.  The spacing in Comment 25 was correct, but it was changed for some reason before it got rolled into a patch.

Also note that these patches have been checked in upstream.

Comment 67

[Milan Sreckovic [:milan] (needinfo for best results)]

Attachment: Take this patch for landing, rather than the approved one above. White space changes only.

Please take this instead of the above, approved patch - white space changes only.

Comment 68

[Milan Sreckovic [:milan] (needinfo for best results)]

(In reply to DRC from comment #66)
> Please note that the DHT patch needs to be reformatted for spacing.  The
> spacing in Comment 25 was correct, but it was changed for some reason before
> it got rolled into a patch.

Sorry about that.  I've attached a modified patch with the correct white space.

Comment 69

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/364bd220cdb7

I landed a whitespace fixup on inbound/m-c since the revised patch was posted after this landed there.
https://hg.mozilla.org/integration/mozilla-inbound/rev/9a9a6324b001

Comment 70

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/edc3765d2780
https://hg.mozilla.org/releases/mozilla-beta/rev/52aaf3194c42
https://hg.mozilla.org/releases/mozilla-esr24/rev/c397c3264c98
https://hg.mozilla.org/releases/mozilla-b2g26_v1_2/rev/52aaf3194c42
https://hg.mozilla.org/releases/mozilla-b2g18/rev/617eb9d9bcc2
https://hg.mozilla.org/releases/mozilla-b2g18_v1_1_0_hd/rev/617eb9d9bcc2

Comment 71

[Carsten Book [:Tomcat]]

(In reply to Ryan VanderMeulen [:RyanVM UTC-5] from comment #69)
> https://hg.mozilla.org/mozilla-central/rev/364bd220cdb7
> 
> I landed a whitespace fixup on inbound/m-c since the revised patch was
> posted after this landed there.
> https://hg.mozilla.org/integration/mozilla-inbound/rev/9a9a6324b001

also landed on central https://hg.mozilla.org/mozilla-central/rev/9a9a6324b001

Comment 72

[Ioana (away)]

(In reply to Michal Zalewski from comment #27)
> To be clear, there are two related info leak bugs. One is fixed by comment
> 21 (http://lcamtuf.coredump.cx/jpeg_leak/), the other should be fixed by
> that last one (http://lcamtuf.coredump.cx/jpeg_leak/turbo-dht.jpg).

Verified the second issue on:
Mozilla/5.0 (X11; Linux x86_64; rv:26.0) Gecko/20100101 Firefox/26.0 (20131125215016)
Mozilla/5.0 (X11; Linux x86_64; rv:27.0) Gecko/20100101 Firefox/27.0 (20131127004001)
Mozilla/5.0 (X11; Linux x86_64; rv:28.0) Gecko/20100101 Firefox/28.0 (20131126052050)

Couldn't verify the first issue because the right-hand images don't get loaded in builds with the fix ("The image cannot be displayed because it contains errors"). They get loaded every time in pre-fix builds.

Comment 73

[Matt Wobensmith [:mwobensmith][:matt:]]

Ioana, regarding the first image - try the same URL in Safari 6.0 for a comparison. Currently, Safari (for me) displays the cross-domain image as image data in the bad JPG, which is something we want to prevent.

I think that it is OK if the new behavior is to block the bad image. 

To verify the fix, confirm that the image either a) doesn't load or b) loads, but does not display as it does in Safari.

If there are any doubts, let me know and I'll verify.

Comment 74

[Michal Zalewski]

Failure to display the image is the correct behavior. The fix causes the library to reject images with duplicate component IDs.

Comment 75

[Michal Zalewski]

Also note that CVE-2013-6629 is assigned to the SOS issue; there is a separate CVE-2013-6630 for the DHT issue.

Comment 76

[Ioana (away)]

Thanks for the details guys! I didn't compare Firefox with other browsers because Safari and Chrome were broken when I tried. IE does work fine here though. Anyway, given comment 74, this bug is fixed on on the Ubuntu builds in comment 72.

I also verified this fix on Windows 7 64bit and Mac OS X 10.9, on Firefox 26.0b10 and the 12/04 Aurora and Nightly.

I will only verify the fix on Firefox 24.2.0 ESR since the builds are not done yet.

Comment 77

[Matt Wobensmith [:mwobensmith][:matt:]]

Verified 24.2.0esr, 2013-12-03.