Bug#496612: ffmpeg: dca decoder crashes on a crafted file

[Alexander E. Patrakov]

Reinhard Tartler wrote:
> "Alexander E. Patrakov" <patrakov at gmail.com> writes:
> > Try to ffplay or convert the attached crash.dts file (not a valid DTS
> > stream, produced by accident when I made a wrong change to my
> > not-yet-released DTS encoder) - ffmpeg segfaults at least on amd64, which
> > may have security implications.
>
> I can reproduce this on i386 as well.
>
> > This crash is not present in the latest SVN version of ffmpeg
> > (although I don't know which revision fixes this), and it also has some
> > correctness fixes that you may want to backport (dcadata.h: revision
> > 14964, dca.c: revisions 14937 and 14917).
>
> I'm very hesitant to backport fixes to lenny now, to be honest. The
> changes to the table in dcadata.h is something which I cannot
> review because I have no idea what it does.

The table in dcadata.h was incorrectly copied from the official specification. 
This fix is vital in order to play back DTS files that use 
perfect-reconstruction 32-band interpolation FIR. The DTS specification looks 
like this (copied and pasted from PDF):

D.8         32-Band Interpolation FIR
D.8.1       Perfect Reconstruction
+1.135985195E-010 -6.022448247E-007 +9.742954035E-007
+7.018770981E-011 -6.628192182E-007 +1.085227950E-006
-1.608403011E-008 -6.982898526E-007 +1.162929266E-006
-5.083275667E-008 -7.020648809E-007 +1.194632091E-006
-1.543309907E-007 -6.767839409E-007 +1.179182050E-006
-3.961981463E-007 -6.262345096E-007 +1.033426656E-006
-7.342250683E-007 -5.564140224E-007 +9.451737242E-007
-3.970030775E-007 +7.003467317E-007 +1.975324267E-006
-4.741137047E-007 +8.419976893E-007 +1.190443072E-006
(continued on the next page)

It is supposed to be read by columns (i.e., +1.135985195E-010, 
+7.018770981E-011, ...), but was initially incorrectly copied by rows (i.e., 
+1.135985195E-010, -6.022448247E-007, ...).

> The fix for the scaling factor looks okay to me if it comes from
> upstream, and for the buffer size adjustment (the patch you're probably
> talking about), I'm testing that right now.

It comes from me (on the basis of encoding and then decoding a known wav file 
with "Surcode DVD DTS" and libdca/ffmpeg, respectively) and was accepted 
upstream in both libdca (which had the "WTF" comment near this scaling on 
2/3, because the spec never mentions it) and ffmpeg.

> Note this upstream comment for the patch of yours:
>
> r14917 | michael | 2008-08-23 15:29:13 +0200 (Sa, 23 Aug 2008) | 3 lines
>
> Increase buffer size to 16384 patch by Alexander E. Patrakov" patrakov
> gmail This fixes a (probably not exploitable) buffer overflow
> (apparently unknown to its author).
>
> do you have evidence that this buffer overflow was exploitable?

No, I sent this because my own DTS encoder produced a file that looked valid 
according to the spec but that ffmpeg didn't accept. I didn't suspect a 
buffer overflow. So the patch to fix the crash on crash.dts is something 
else.

-- 
Alexander E. Patrakov