Bug 599147 - Libvpx has memory access overruns due to lack of motion vector clipping on some block types
Libvpx has memory access overruns due to lack of motion vector clipping on so...
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: libvpx (Show other bugs)
13
All Linux
low Severity high
: ---
: ---
Assigned To: Tom "spot" Callaway
Fedora Extras Quality Assurance
: Security
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2010-06-02 14:27 EDT by Gregory Maxwell
Modified: 2010-07-05 00:30 EDT (History)
2 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2010-06-28 14:59:46 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)
example trigger (518.16 KB, application/octet-stream)
2010-06-02 16:26 EDT, Gregory Maxwell
no flags Details
The promised test output (2.92 KB, application/x-bzip2)
2010-06-03 14:21 EDT, Jan Lieskovsky
no flags Details

  None (edit)
Description Gregory Maxwell 2010-06-02 14:27:28 EDT
When libvpx was being discussed on fedora-devel I mentioned that there are "security relevant bugs", and cautioned against being overly hasty. 


This was ignored and fedora has shipped libvpx in the repositories, and users who have installed it have it exposed to the internet via konqueror.

I'll attach a demonstration file that causes ivfdec to segfault on x86_64. 
Reproduction there is as simple as ./ivfdec fuzz.ivf -o /dev/null.   

If this file doesn't crash on your particular system you can find another test case easily enough with:

zzuf --include any_input_ivf.ivf -s 0:1000 -r 0.0001 ./ivfdec any_input_ivf.ivf -o /dev/null

This hasn't yet been fixed in upstream libvpx because fixing it correctly breaks the bitstream. 

The browser vendors (e.g. firefox) are shipping their demo copies with patches, but because of the aforementioned bit-stream issue it causes corruption in some decodes.

There is another fix which retains compatibility with the existing encoder but which still leaves open inconsistent behaviour from other decoders as well as increasing memory usage and lowering performance.

(These patches are available on the webm development mailing list)
Comment 1 Tom "spot" Callaway 2010-06-02 14:44:07 EDT
Can you point to the upstream bug ticket or the thread where this is discussed (with patches)?

Thanks in advance.
Comment 2 Gregory Maxwell 2010-06-02 14:57:15 EDT
Sorry, I intended to link the thread and got a little send-happy:

https://groups.google.com/a/webmproject.org/group/codec-devel/browse_frm/thread/ff90bd82d0369b96/79d4c40ea78db91b?tvc=1&q=timothy#79d4c40ea78db91b
Comment 3 Tom "spot" Callaway 2010-06-02 15:32:16 EDT
Also, I do not believe you attached the demonstration file. :)
Comment 4 Gregory Maxwell 2010-06-02 16:26:33 EDT
Created attachment 419154 [details]
example trigger
Comment 5 Jan Lieskovsky 2010-06-03 14:17:25 EDT
Hi Gregory,

  thank you for your report. Just checking -- under "./ivfdec"
above you refer to which decoder? This one:
  [1] http://www.webmproject.org/tools/vp8-sdk/example_ivfdec.html

(wondering, because was trying this one:
  [2] http://www.webmproject.org/tools/vp8-sdk/example_simple_decoder.html

and couldn't reproduce the crashes even when trying the "zzuf" command line
above on x86_64 libvpx-0.9.0-6.fc12 [the version you reported against
i.e. libvpx-0.9.0-6.fc13 should be the same]).

Will attach the output I got and try [2] yet.

Thanks && Regards, Jan.
--
Jan iankko Lieskovsky / Red Hat Security Response Team
Comment 6 Jan Lieskovsky 2010-06-03 14:21:02 EDT
Created attachment 419473 [details]
The promised test output

Was run with command:

   zzuf --include fuzz.ivf -s 0:1000 -r 0.0001 ./sample_ivfdec fuzz.ivf /dev/null > /tmp/`rpm -q libvpx`_output.txt

What I am doing wrong here?

Thanks, Jan.
Comment 7 Tom "spot" Callaway 2010-06-03 14:45:16 EDT
Jan, I was able to reproduce a segfault on my Fedora 13 x86_64 system, using the ivfdec binary in the libvpx-utils package. Applying the patch from the thread in Comment 2 resolves the issue for the time being, so I have already done builds for all targets with it.

I was about to push updates. Do you have any reason for me not to?
Comment 8 Jan Lieskovsky 2010-06-03 15:50:33 EDT
(In reply to comment #7)
> Jan, I was able to reproduce a segfault on my Fedora 13 x86_64 system, using
> the ivfdec binary in the libvpx-utils package.

Ah, thanks, Tom, ok.

 Applying the patch from the
> thread in Comment 2 resolves the issue for the time being, so I have already
> done builds for all targets with it.

You mean this one:?
  [1] https://bugzilla.mozilla.org/show_bug.cgi?id=566987#c6
  [2] https://groups.google.com/a/webmproject.org/group/codec-devel/browse_thread/thread/ff90bd82d0369b96
  [3] https://groups.google.com/a/webmproject.org/group/codec-devel/attach/79d4c40ea78db91b/0001-Test-commit-for-a-version-of-the-SPLITMV-bounds-patc.patch?part=3

FWICT [1] suggests this is only temporary solution, as it may break
some valid streams. Hopefully, we should investigate the issue in
more depth yet, watch && contribute to the upstream discussion regarding
this && build only the final conclusion patch (not causing regressions).

Tom, Gregory, what's your opinion?

> 
> I was about to push updates. Do you have any reason for me not to?
Comment 9 Tom "spot" Callaway 2010-06-03 15:57:58 EDT
The patch in [3] is explicitly stated to not break valid streams, just that it is ugly. 

I'm inclined to push it as an update now, if for no other reason than to eliminate potential browser crashes once more browsers start enabling VP8 support.

If/when a "final conclusion" patch emerges, we can move to it (likely with a new upstream release of libvpx).
Comment 10 Gregory Maxwell 2010-06-03 16:02:04 EDT
The patch in that thread does not break existing bitstreams. See comment 3 on the original thread or Tim's comments that you linked to in [1].

The reason it isn't yet the final upstream solution is that it permanently makes all decoders do extra work and also makes the behaviour inconsistent between modes, which is sure to cause implementation bugs.

Tim (and me, and a lot of other people) would really like google to break the bitstream to adopt a proper fix. Unfortunately as time goes on this seems less and less likely. (Their resistance to doing this comes from that fact that they have made commitments, plus youtube has already transcoded something like few million files)  If google doesn't change the bitstream than the patch is the only option.

My recommendation would have been to not ship libvpx at all until this was resolved. Seeing as how thats no longer an option,  shipping the fix that is compatible with the existing encoder is strictly superior to doing nothing.  It closes a potential security vulnerability, and doesn't create an _additional_ incompatibility over what would exist if the bitstream is changed.

I asked Tim for his thoughts and he said "I mean, it's better than nothing. At this rate, it's going to become the de-facto standard."
Comment 12 Tom "spot" Callaway 2010-06-28 14:59:46 EDT
The pending 0.9.1 update resolves this. Closing out the ticket.

(I would do it normally, via bodhi, but it doesn't handle security restricted bugs, and no one can seem to figure out how to uncheck that box.)

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