Bug 614643 (CVE-2009-3995, CVE-2009-3996, CVE-2010-2546, CVE-2010-2971)

Created attachment 431930 [details]
patch from Debian to correct the issues

This patch was pulled from libmikmod_3.1.11-6+lenny1.diff, which is from DSA 2071-1.

Created libmikmod tracking bugs for this issue

Affects: fedora-all [bug 614650]

(In reply to comment #2)
> Created an attachment (id=431930) [details]
> patch from Debian to correct the issues
> 
> This patch was pulled from libmikmod_3.1.11-6+lenny1.diff, which is from DSA
> 2071-1.    

This seem to contain following two upstream CVS commits:

http://mikmod.cvs.sourceforge.net/viewvc/mikmod/libmikmod/loaders/load_it.c?r1=1.3&r2=1.4
http://mikmod.cvs.sourceforge.net/viewvc/mikmod/libmikmod/loaders/load_ult.c?r1=1.2&r2=1.3

However, load_it.c fix seems to be incomplete and possibly mis-placed too.  IT_ProcessEnvelope macro is used do common processing on volume, panning and pitch envelopes.  It writes to volenv/panenv/pitenv arrays with ENVPOINTS size using volpts/panpts/pitpts as an upper bound (those are unsigned char variables, with maximum value of 255), for loop following the check is hence not the first place where name##env arrays are written to.  I don't see why upstream fix only adds check for vol and not pan and pit, and why the check is not done in IT_LoadEnvelope already.

Reading the Secunia advisory, it seems they've treated all tree cases as issues:

"... when parsing an instrument containing a column, panning, or pitch envelope with more than ENVPOINTS (32) points ..."

(I guess this should say: containing a *volume*, panning, or pitch envelope)

(In reply to comment #5)
> It writes to volenv/panenv/pitenv arrays with ENVPOINTS size using
> volpts/panpts/pitpts as an upper bound (those are unsigned char variables,
> with maximum value of 255), for loop following the check is hence not the
> first place where name##env arrays are written to.

Additionally, values written to name##env are read from name##tick and name##node, which are of size ITENVCNT (25).

(In reply to comment #5)

> However, load_it.c fix seems to be incomplete and possibly mis-placed too. 
> IT_ProcessEnvelope macro is used do common processing on volume, panning and
> pitch envelopes.  It writes to volenv/panenv/pitenv arrays with ENVPOINTS size
> using volpts/panpts/pitpts as an upper bound (those are unsigned char
> variables, with maximum value of 255), for loop following the check is hence
> not the first place where name##env arrays are written to.  I don't see why
> upstream fix only adds check for vol and not pan and pit, and why the check is
> not done in IT_LoadEnvelope already.

Secunia reproducers crash libmikmod with upstream patch for CVE-2009-3995, so that fix is really bogus.

Created attachment 433716 [details]
Do checks in IT_LoadEnvelope

Moves name##pts check to IT_LoadEnvelope.  Uses ITENVCNT as an upper bound.  It is less than name##env size (ENVPOINTS), but only ITENVCNT items are read from file to name##node and name##tick, so trying to user higher upper bound should over-read buffer.  Please correct me if I'm missing something.

(In reply to comment #11)
> Created an attachment (id=433716) [details]
> Do checks in IT_LoadEnvelope

https://sourceforge.net/tracker/?func=detail&aid=3033086&group_id=40531&atid=428227

(In reply to comment #12)

> https://sourceforge.net/tracker/?func=detail&aid=3033086&group_id=40531&atid=428227    

CVE-2010-2546 was assigned to the incorrect CVE-2009-3995 fix.

As updates for Fedora or Red Hat Enterprise Linux mikmod packages were not released yet, updates containing complete fix will not need to mention CVE-2010-2546.

The water is even muddier... MITRE also assigned CVE-2010-2971 due to the incomplete fix of CVE-2009-3995.  For clarification, MITRE's descriptions of the problems are as follows:

CVE-2010-2546:

Multiple heap-based buffer overflows in loaders/load_it.c in
libmikmod, possibly 3.1.12, might allow remote attackers to execute
arbitrary code via (1) crafted samples or (2) crafted instrument
definitions in an Impulse Tracker file, related to panpts, pitpts, and
IT_ProcessEnvelope.  NOTE: some of these details are obtained from
third party information.  NOTE: this vulnerability exists because of an
incomplete fix for CVE-2009-3995.

CVE-2010-2971:

loaders/load_it.c in libmikmod, possibly 3.1.12, does not properly
account for the larger size of name##env relative to name##tick and
name##node, which allows remote attackers to trigger a buffer
over-read and possibly have unspecified other impact via a crafted
Impulse Tracker file, a related issue to CVE-2010-2546.  NOTE: this
issue exists because of an incomplete fix for CVE-2009-3995.


Neither of these CVEs need to be mentioned due to the fact that we did not improperly fix CVE-2009-3995.

(In reply to comment #16)
> The water is even muddier... MITRE also assigned CVE-2010-2971 due to the
> incomplete fix of CVE-2009-3995.

Not really.  Mitre decided to give separate CVE CVE-2010-2971 to the over-read issue mentioned above in comment #6.  Though the over-read is rather limited and not too likely to have any real impact.

This issue has been addressed in following products:

  Red Hat Enterprise Linux 3
  Red Hat Enterprise Linux 4
  Red Hat Enterprise Linux 5

Via RHSA-2010:0720 https://rhn.redhat.com/errata/RHSA-2010-0720.html