RE: CR: 249097 - Security fix - urgent CR requested

This will be going into 
	hxclient_2_0_4_cayenne
	hxclient_3_1_0_atlas
and	HEAD

> -----Original Message-----
> From: Gregory Wright [mailto:[email protected]]
> Sent: Friday, August 28, 2009 4:23 PM
> To: Steve Blanding
> Cc: [email protected]; [email protected]
> Subject: Re: [helix-client-dev] CR: 249097 - Security fix - urgent CR
> requested
> Importance: High
> 
> Change looks good.
> 
> You will need to come up with a list of branches for it to go into.
> HEAD and 310Atals at least and perhaps some Cayenne branches as
> needed. Talk to the player teams.
> 
> --greg.
> 
> 
> On Aug 28, 2009, at 3:32 PM, Steve Blanding wrote:
> 
> > We have received an outside report of a buffer overrun exploit in
> > Helix.  I have verified the existence of the exploit and have coded
> > and tested a fix.
> >
> > The exploit is possible due to a hard-coded array that is used when
> > processing ASM rules.  The array can accommodate only 256 rules
> > (which under normal circumstances should be more than sufficient)
> > but there were no safeguards in the code to prevent someone from
> > specifying more than 256 rules and overrunning memory.
> >
> > The simplest fix is to check the number of rules found and disallow
> > files that contain more than 256 rules.
> >
> > As part of the fix, I have also removed the magic number and
> > replaced it with a #define to explicitly call out what the number
> > represents.
> >
> > Index: rtspclnt.cpp
> > ===================================================================
> > RCS file: /cvsroot/protocol/rtsp/rtspclnt.cpp,v
> > retrieving revision 1.148.2.51.2.8
> > diff -u -w -r1.148.2.51.2.8 rtspclnt.cpp
> > --- rtspclnt.cpp  15 Jul 2009 20:34:39 -0000
> > 1.148.2.51.2.8
> > +++ rtspclnt.cpp               28 Aug 2009 21:11:22 -0000
> > @@ -182,7 +182,7 @@
> >      { "ANNOUNCE",       ANNOUNCE    }
> >  };
> >
> > -
> > +#define MAX_ASM_RULES 256
> >
> >
> >  /*
> > @@ -10722,9 +10722,19 @@
> >              ASMRuleBook     rules((char*)pRuleBuf->GetBuffer());
> >
> >              unRules = rules.GetNumRules();
> > +                 if (unRules > MAX_ASM_RULES)
> > +                 {
> > +                             // If we ever exceed this many rules
> > then we have bad data and we should fail.
> > +                HX_RELEASE(pRuleBuf);
> > +                             HX_RELEASE(pFileHeader);
> > +                             HX_RELEASE(pBandwidth);
> > +                             HX_RELEASE(pHeader);
> > +                             HX_ASSERT(FALSE);
> > +                             return FALSE;
> > +                 }
> >
> >              // get subscriptions for this bandwidth
> > -            HXBOOL bSubInfo[256];
> > +            HXBOOL bSubInfo[MAX_ASM_RULES];
> >              UINT16 unRuleNum = 0;
> >
> >              IHXValues* pValues = new CHXHeader();
> > @@ -10787,9 +10797,18 @@
> >          ASMRuleBook     rules((char*)pRuleBuf->GetBuffer());
> >
> >          unRules = rules.GetNumRules();
> > +             if (unRules > MAX_ASM_RULES)
> > +             {
> > +                 // If we ever exceed this many rules then we have
> > bad data and we should fail.
> > +                 HX_RELEASE(pRuleBuf);
> > +                 HX_RELEASE(pFileHeader);
> > +                 HX_RELEASE(pBandwidth);
> > +                 HX_ASSERT(FALSE);
> > +                 return FALSE;
> > +             }
> >
> >          // get subscriptions for this bandwidth
> > -        HXBOOL bSubInfo[256];
> > +        HXBOOL bSubInfo[MAX_ASM_RULES];
> >
> >          IHXValues* pValues = new CHXHeader();
> >          pValues->AddRef();
> > _______________________________________________
> > Helix-client-dev mailing list
> > [email protected]
> > http://lists.helixcommunity.org/mailman/listinfo/helix-client-dev