Re: CN: 249097 - Security fix - urgent CR requested

This patch has been merged to 205Cay and dev. tested there.

-- 
Daniel Yek.



Steve Blanding wrote:
>
> Done.  Thanks all.
>
>  
>
> *From:* Rishi Mathew [mailto:[email protected]]
> *Sent:* Friday, August 28, 2009 4:31 PM
> *To:* Gregory Wright; Steve Blanding
> *Cc:* [email protected]; [email protected]
> *Subject:* Re: [helix-client-dev] CR: 249097 - Security fix - urgent 
> CR requested
>
>  
>
> In addition, please commit also to 347_atlas, 348_atlas, 349_atlas, 
> 210_cays, 150_cay.
>
> Thanks,
> Rishi.
>
> At 04:23 PM 8/28/2009, Gregory Wright wrote:
>
> 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();
>