Bug#864195: libopenmpt: Security updates libopenmpt-0.2.7386-beta20.3-p7 available

[Salvatore Bonaccorso]

Hi James,

On Sun, Jun 25, 2017 at 11:09:05PM +0100, James Cowgill wrote:
> Hi security team,
> 
> On 07/06/17 10:45, James Cowgill wrote:
> > On 05/06/17 07:03, Jörn Heusipp wrote:
> >> Source: libopenmpt Version: 0.2.7386~beta20.3-3 Severity: 
> >> important Tags: upstream
> >> 
> >> Dear Maintainer,
> >> 
> >> A couple of security-related fixes have been released upstream
> >> as version 0.2.7386-beta20.3-p7. See 
> >> https://lib.openmpt.org/libopenmpt/md_announce-2017-06-02.html
> >> 
> >> These most importantly fix a couple of possible crashes which
> >> can be triggered by maliciously modified or malformed or
> >> truncated module files as well as denial-of-service through hangs
> >> or excessive CPU consumption which can also be triggered
> >> maliciously modfied or malformed or truncated module files.
> > 
> > I've had a look at the patches now and this is what I think:
> > 
> > p1-division-by-zero-in-tempo-calculation.patch 
> > p2-infinite-loop-in-plugin-routing.patch 
> > p6-invalid-memory-read-when-applying-nnas-to-effect-plugins.patch
> > 
> > These three are clearly denial-of-service by malicious module file 
> > and should be fixed in stretch. However, I don't think the first 
> > two are "serious" because they're just denial-of-service bugs in a 
> > library almost exclusively used on end user machines (as opposed
> > to eg remote code execution). I don't understand patch p6 well
> > enough to say how serious it is (depends on where the invalid
> > pointer being dereferenced comes from).
> > 
> > p3-excessive-cpu-consumption-on-malformed-files-dmf-mdl.patch 
> > p5-excessive-cpu-consumption-on-malformed-files-ams.patch
> > 
> > Are these actually security bugs? As long as the code finishes in a
> > reasonable amount of time and produces the right results, then 
> > there's not much harm in leaving the code as it is.
> > 
> > p4-theoretical-null-pointer-dereference-during-out-of-memory-while-error-handling.patch
> >
> >
> > 
> I don't think this is a security bug. It requires malloc to fail,
> > and the chances of that happening on Linux are very small. If that 
> > does occur, you're likely to be killed by the OOM killer anyway.
> > 
> > I also note that the C++ standard _requires_ std::exception::what 
> > to return a non-null value so a very intelligent compiler could 
> > legitimately remove the null check (I doubt GCC does this though).
> > 
> > p7-race-condition-in-multi-threaded-use-it.patch
> > 
> > I also don't think this is a security bug (at least on Linux). 
> > Looking at the glibc sources, the internal tzdata state is 
> > protected by a mutex so the only risk here is that localtime will 
> > return some garbage time values. Since the string generated by
> > this function is only going to be shown to the user, nothing that
> > bad will happen in this case. Finally, it relies on a
> > multithreaded client application loading 2 modules at the same time
> > which seems unlikely to me.
> 
> I decided later in this thread that all the patches except p4 and p7
> should be fixed in stretch. I opened a stretch-pu bug here: #865355. I
> it OK for these to be fixed as a stretch update or should they be
> handled by you?

Yes it is perfectly fine to be handled via a point release.

Regards and thanks for your work!
Salvatore