Bug#782831: libvorbis: use of non-initialized variable leads to SIGSEV in dhewm3

[Martin Steghöfer]

Dear Tobi,

Thank you for the bug report and your analysis of the problem.

The error handling is one thing. A library should be robust to failures 
and not crash the whole product. The crashing doesn't happen on other 
distros because for bug #762571 we backported an upstream change that 
fixes an infinite loop when seeking to 0 and that new code is apparently 
less robust to internal failures. Since the code comes from upstream, 
their current SVN head has probably the same problem.

But on the other hand I don't think there is a reason for the internal 
function _get_next_page to fail. I think the function ov_pcm_seek_page 
is allowed to assume that under the given circumstances the call to 
_get_next_page has to succeed. So mitigating the problem by making the 
code more robust (as your patch does) would avoid the crash, but the 
call to ov_pcm_seek_page would still fail without justification and the 
playback would have to be aborted.

That's why I tried to find the underlying problem. Unfortunately, the 
fact that the only known way to trigger the bug was via "dhewm3" 
required me to build that application and even dig out a copy of Doom 3, 
which is apparently required to run the test case. Finally I was able to 
extract the data that triggered the crash and reproduce the crash in a 
simple executable without "dhewm3". Further debugging revealed that the 
problem can be triggered by doing two subsequent seeks to 0 on any very 
short vorbis file (not only the one extracted from "dhewm3" / Doom 3) - 
which is great because this way we have an example file that can be 
posted here without copyright trouble.

For now that's as far as I got. But at least the bug is isolated now 
(see attached files; compile vorbistest.c against libvorbis-dev and 
execute it with the path to silence-short.ogg as first parameter). When 
I have more time, I can do further debugging to find the root of the 
problem.

Cheers,
Martin




El 18-04-2015 a les 14:31, Tobias Frost ha escrit:
> Package: libvorbis-dev
> Version: 1.3.4-2
> Severity: normal
>
> Dear Maintainer,
>
> Please read for a background:  https://github.com/dhewm/dhewm3/issues/104
>
> Short: dhewm3 calls ov_pcm_seek_page(), where an object of ogg_page (og) is created
> on the heap, which seems should be initialized by the call to _get_next_page().
> However the call to this function is not successful, returnin OV_EOF (-2) and
> the result is that ogg_page is not initialized but still later used, even if the code
> recognized that bisection fails (bisec==-1). However, it still accesses it in
> the call to  ogg_page_serialno(&og) leading to a SEGV.
>
> A special guard for this, first initializing og.header with NULL and checking for it later
> makes the SIGSE'm unsure if this fix is "sane". With that disclaimer, here's the patch I used:
>
> diff -Naur libvorbis-1.3.4/lib/vorbisfile.c libvorbis-1.3.4_/lib/vorbisfile.c
> --- libvorbis-1.3.4/lib/vorbisfile.c    2015-04-18 14:28:43.000000000 +0200
> +++ libvorbis-1.3.4_/lib/vorbisfile.c   2015-04-18 14:04:34.262733024 +0200
> @@ -1442,6 +1442,7 @@
>       ogg_int64_t best=-1;
>   
>       ogg_page og;
> +    og.header = 0;
>       /* bisection loop */
>       while(begin<end){
>         ogg_int64_t bisect;
> @@ -1550,6 +1551,11 @@
>            bisection would 'fail' because our search target was before the
>            first PCM granule position fencepost. */
>   
> +      if (!og.header) {
> +         result = -1;
> +         goto seek_error;
> +      }
> +
>         if(begin == vf->dataoffsets[link] &&
>            ogg_page_serialno(&og)==vf->serialnos[link]){
>   
>
>
> Please let me know how I can help...
>
> --
> tobi
>
>
> -- System Information:
> Debian Release: 8.0
>    APT prefers testing-updates
>    APT policy: (500, 'testing-updates'), (500, 'testing')
> Architecture: amd64 (x86_64)
> Foreign Architectures: i386
>
> Kernel: Linux 3.16.0-4-amd64 (SMP w/4 CPU cores)
> Locale: LANG=de_DE.utf8, LC_CTYPE=de_DE.utf8 (charmap=UTF-8)
> Shell: /bin/sh linked to /bin/dash
> Init: systemd (via /run/systemd/system)
>
> Versions of packages libvorbis-dev depends on:
> ii  libogg-dev      1.3.2-1
> ii  libvorbis0a     1.3.4-2
> ii  libvorbisenc2   1.3.4-2
> ii  libvorbisfile3  1.3.4-2
>
> libvorbis-dev recommends no packages.
>
> libvorbis-dev suggests no packages.
>
> -- no debconf information
>

-------------- next part --------------
A non-text attachment was scrubbed...
Name: vorbistest.c
Type: text/x-csrc
Size: 556 bytes
Desc: not available
URL: <http://lists.alioth.debian.org/pipermail/pkg-xiph-maint/attachments/20150420/90a75f9a/attachment.c>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: silence-short.ogg
Type: video/ogg
Size: 3749 bytes
Desc: not available
URL: <http://lists.alioth.debian.org/pipermail/pkg-xiph-maint/attachments/20150420/90a75f9a/attachment.ogv>