966044 - Unconditional buffer overflow in EbmlComposer::GenerateHeader()

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[Eric Faust [:efaust]]

Attachment: fix

We declare a stack buffer of size 8, then copy a string of length 8 into it. The null character smashes the stack.

Comment 1

[cajbir (:cajbir)]

Comment on attachment 8368246 [details] [diff] [review]
fix

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

Changes look good but needs gecko_fixes.patch to be updated for r+.

::: media/libmkv/WebMElement.c
@@ +55,5 @@
>    return t ^ r;
>  }
>  
>  void writeVideoTrack(EbmlGlobal *glob, unsigned int trackNumber, int flagLacing,
> +                     const char *codecId, unsigned int pixelWidth, unsigned int pixelHeight,

gecko_fix.patch also needs to be updated for this file.

::: media/libmkv/WebMElement.h
@@ +24,1 @@
>                       double frameRate);

You'll need to update gecko_fix.patch for these changes. :rillian may be able to advise on how this was generated.

Comment 2

[Ralph Giles (:rillian)]

The const-ification is cleaner than gecko-fix.patch. so please put the change in a new patch file and apply it from media/libmkv/update.sh. The idea is to make the patches we carry against libraries with an upstream explicit so it's easy to upgrade the underlying code.

If you're willing to submit this upstream, let me know. Otherwise I can do it.

Comment 3

[Eric Faust [:efaust]]

Attachment: fix without modifying third party libraries

Comment 4

[Ralph Giles (:rillian)]

I liked the other better!

Comment 5

[Eric Faust [:efaust]]

Attachment: first fix...fixed

Comment 6

[Ralph Giles (:rillian)]

Comment on attachment 8368297 [details] [diff] [review]
first fix...fixed

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

Please put the const changes in a separate const.patch instead of in gecko_fix.patch.

Comment 7

[Eric Faust [:efaust]]

Attachment: fix of fixed first fix.

Comment 8

[Eric Faust [:efaust]]

Attachment: fix of first fix fixed, fixed to add patch containing fix.

Comment 9

[Ralph Giles (:rillian)]

Comment on attachment 8368306 [details] [diff] [review]
fix of first fix fixed, fixed to add patch containing fix.

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

I think that fixed it.

Comment 11

[Eric Faust [:efaust]]

(In reply to Ralph Giles (:rillian) from comment #9)
> Comment on attachment 8368306 [details] [diff] [review]
> fix of first fix fixed, fixed to add patch containing fix.
> 
> Review of attachment 8368306 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> I think that fixed it.

I could weep.

https://hg.mozilla.org/integration/mozilla-inbound/rev/7c3373499773

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/7c3373499773

Comment 13

[cajbir (:cajbir)]

Comment on attachment 8368273 [details] [diff] [review]
fix without modifying third party libraries

Review no longer needed.