Re: [Iaxclient-devel] possible security hole in iax.c

[Alexander V. <ava...@vo...>]

Hi Pete,

No patches yet, maybe it would be even easier to just edit the code.
Currently I am studying the newly introduced video code, the way packet
size is verified there is also not clean. I will make a patch after I
finish examining the mini video header-related code.

Alex


Peter Grayson wrote:
> Hi Alexander
>
> Do you have patches that addresses these two issues?
>
> Pete
>
> On Tue, Apr 22, 2008 at 3:09 PM, Alexander Vassilev
> <ava...@vo...> wrote:
>   
>> Hi all,
>>
>>  If you remember from the past, there was a security issue with
>>  iax_net_process() tolerating frames that are amaller in size than the
>>  frame header itself. There were checks, but they only logged warnings,
>>  and didnt prevent a possible exploit. There was a security advisory on
>>  this, on securityfocus or somewhere else, not sure. The fix was to add a
>>  return statement if the checks catch a malformed packet. Now, looking at
>>  the code, I see the following:
>>  code from iax.c:
>>
>>  struct iax_event *iax_net_process(unsigned char *buf, int len, struct sockaddr_in *sin)
>>   {
>>         struct ast_iax2_full_hdr *fh = (struct ast_iax2_full_hdr *)buf;
>>         struct ast_iax2_mini_hdr *mh = (struct ast_iax2_mini_hdr *)buf;
>>         struct ast_iax2_video_hdr *vh = (struct ast_iax2_video_hdr *)buf;
>>         struct iax_session *session;
>>
>>         if (ntohs(fh->scallno) & IAX_FLAG_FULL) {
>>         /* Full size header */
>>                 if ((size_t)len < sizeof(struct ast_iax2_full_hdr)) {
>>                 DEBU(G "Short header received from %s\n", inet_ntoa(sin->sin_addr));
>>                 IAXERROR "Short header received from %s\n", inet_ntoa(sin->sin_addr));
>>                 return NULL;
>>         }
>>
>>  ...
>>  Note that there is no check if we actually have the 2 bytes in hte
>>  buffer that represent fh->scallno. What happens if we receive a frame
>>  with size 1 byte?
>>
>>  I also noticed an erroneous comment in this function - this one is not a
>>  security risk :) but would be good if its fixed
>>  code from iax.c:
>>
>>  3236            /* Miniature, voice frame */
>>  3237            if ((vh->zeros == 0) && (ntohs(vh->callno) & 0x8000))
>>
>>
>>
>>  This is actually a miniature video frame
>>
>>  Best regards
>>
>>  Alexander Vassilev
>>  Senior software engineer
>>  VoipGATE S.A.
>>
>>
>>  -------------------------------------------------------------------------
>>  This SF.net email is sponsored by the 2008 JavaOne(SM) Conference
>>  Don't miss this year's exciting event. There's still time to save $100.
>>  Use priority code J8TL2D2.
>>  http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone
>>  _______________________________________________
>>  Iaxclient-devel mailing list
>>  Iax...@li...
>>  https://lists.sourceforge.net/lists/listinfo/iaxclient-devel
>>
>>     
>
>