Re: [sipX-dev] proposed fix for XPB-1023
Scott,
Revised the change accordingly. See attched patch.
cheers
Huijun
Scott Lawrence <slawrence@xxxxxxxxxxx> wrote:
On Wed, 2007-04-11 at 15:03 -0400, Milner Downs wrote:
> Hi all,
>
> Here is the root cause of the issue XPB-1023.
>
> In a misformatted message, XPB-1023 as an example, in
> SdpBody,codecCount might not match the media type count.
> In SdpBody::getBestAudioCodecs, it allocates an array to hold the
> codec data with the size of codecCount. Then later it tries
> to fill/delete the array with size of mediaTypeCount. If codecCount <
> meidaTypeCount, then it tries to access some memory which does not
> belong to it, and caused the crash.
>
> The proposed fix is to
adjust the array size if there is a mismatch
> between codecCount and mediaTypeCount. To make sure the array is big
> enough to hold the data being processed.
>
> Patch is attached. Have tested the fix. It worked fine.
>
> Please let me know if the fix is acceptable.
Is there some reason why we shouldn't get
max(numAudioTypes,numVideoTypes) and then do all the comparisons and
allocation based on that?
--
Scott Lawrence tel:+1-781-938-5306;ext=162 or sip:slawrence@xxxxxxxxxxx
sipXpbx project coordinator - SIPfoundry http://www.sipfoundry.org/sipX
Chief Technology Officer - Pingtel Corp. http://www.pingtel.com/
The best gets better. See why everyone is raving about the
All-new Yahoo! Mail.
Index: sipXtackLib/src/net/SdpBody.cpp
===================================================================
--- sipXtackLib/src/net/SdpBody.cpp (revision 9324)
+++ sipXtackLib/src/net/SdpBody.cpp (working copy)
@@ -50,6 +50,8 @@
#define SDP_IP4_ADDRESS_TYPE "IP4"
#define NTP_TO_EPOCH_DELTA 2208988800UL
+#define MAX(X,Y) ((X) > (Y) ? (X) : (Y))
+
// STATIC VARIABLE INITIALIZATIONS
/* //////////////////////////// PUBLIC //////////////////////////////////// */
@@ -898,7 +900,17 @@
getMediaPayloadType(mediaVideoIndex, MAXIMUM_MEDIA_TYPES,
&numVideoTypes, videoPayloadTypes);
-
+
+ // This is to handle the case that a mis-formatted message could
have codecCount
+ // not matching medieType count, we want to make sure we allocated
enough space for
+ // the array to "forgive" or "tolerate" this case.
+ int max = MAX(numAudioTypes,numVideoTypes);
+ if(localRtpCodecs.getCodecCount()<max)
+ {
+ delete [] codecsInCommonArray;
+ codecsInCommonArray = new SdpCodec*[max];
+ memset(codecsInCommonArray, 0, sizeof(SdpCodec*)*max);
+ }
getCodecsInCommon(numAudioTypes,
numVideoTypes,
audioPayloadTypes,
