Menu
▾
▴
iaxclient-devel
|
From: Alexander V. <ava...@vo...> - 2008-04-22 19:09:32
|
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.
|
|
From: Peter G. <jpg...@gm...> - 2008-04-22 19:16:14
|
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
>
|
|
From: Alexander V. <ava...@vo...> - 2008-04-22 19:36:12
Attachments:
avasilev.vcf
|
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
>>
>>
>
>
|
|
From: Alexander V. <ava...@vo...> - 2008-04-22 21:33:20
Attachments:
iax.c.patch
avasilev.vcf
|
Hi all, I am attaching a patch that fixes the forementioned problems. The code that was added to handle mini video frames also has a problem - in case the received frame is a mini video frame, a check is done only against the size of an audio miniheader, which is smaller, so a vulnerability similar to the one from Advisory ID: CORE-2006-0327 (coresecurity.com) is again present in iaxclient. This patch should fix it. Best regards Alex |
|
From: Alexander V. <ava...@vo...> - 2008-04-22 21:49:16
Attachments:
iax.c.patch
avasilev.vcf
|
Hmm, seems the patch was broken, here is a newly generated one that works Regards Alex Alexander Vassilev wrote: > Hi all, > > I am attaching a patch that fixes the forementioned problems. The code > that was added to handle mini video frames also has a problem - in case > the received frame is a mini video frame, a check is done only against > the size of an audio miniheader, which is smaller, so a vulnerability > similar to the one from Advisory ID: CORE-2006-0327 (coresecurity.com) > is again present in iaxclient. This patch should fix it. > > Best regards > Alex > > > ------------------------------------------------------------------------- > 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 |
|
From: Peter G. <jpg...@gm...> - 2008-04-23 14:46:49
Attachments:
iax-security3.patch
|
Hi Alex, On Tue, Apr 22, 2008 at 6:14 PM, Alexander Vassilev <ava...@vo...> wrote: > Actually, in the updated patch I didn't include one change, so here is again > an update, hopefully the last one I've reviewed the latest patch. Thank you so much for sending it! It certainly addresses real issues. There were a couple problems with the patch. A trivial problem was that the whitespace was inconsistent. A more important problem was that a mask was wrong: - /* audio frame */ - session = iax_find_session(sin, ntohs(fh->scallno), 0, 0); + /*its a mini audio frame */ + session = iax_find_session(sin, ntohs(mh->callno) & ~0x800, 0, 0); I think you meant ~0x8000. This mask actually isn't necessary though because we've already determined that the full flag is 0. I am attaching an updated patch that I think resolves these two issues. I also moved the "No session?" failure code to the bottom of the function to reduce duplication. Please let me know if you think this updated patch is okay. Thanks! Pete |
|
From: Alexander V. <ava...@vo...> - 2008-04-22 22:14:49
Attachments:
iax.c.newpatch
avasilev.vcf
|
Actually, in the updated patch I didn't include one change, so here is again an update, hopefully the last one Best regards Alex Alexander Vassilev wrote: > Hmm, seems the patch was broken, here is a newly generated one that works > > Regards > Alex > > > Alexander Vassilev wrote: >> Hi all, >> >> I am attaching a patch that fixes the forementioned problems. The code >> that was added to handle mini video frames also has a problem - in case >> the received frame is a mini video frame, a check is done only against >> the size of an audio miniheader, which is smaller, so a vulnerability >> similar to the one from Advisory ID: CORE-2006-0327 (coresecurity.com) >> is again present in iaxclient. This patch should fix it. >> >> Best regards >> Alex >> >> >> ------------------------------------------------------------------------- >> >> 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 > > ------------------------------------------------------------------------- > 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 |
|
From: Alexander V. <ava...@vo...> - 2008-04-23 15:07:06
Attachments:
avasilev.vcf
|
Hello Pete, > Please let me know if you think this updated patch is okay. > Yes, I have mistaken the mask, and yes, it is actually not necessary :) Also thanks for removing the duplicate code, actually I followed the pattern of my code and there I log the type of frame for which the session was not found, hence the duplication here. So you can commit the patch. Thank you! Alex |
|
From: Peter G. <jpg...@gm...> - 2008-04-23 15:35:08
|
Hi Alex, Thank you for your help with this patch. I have committed the changes to trunk. I will be merging this change along with several others into the 2.1 branch in the near future. Pete On Wed, Apr 23, 2008 at 11:06 AM, Alexander Vassilev <ava...@vo...> wrote: > Hello Pete, > > > > Please let me know if you think this updated patch is okay. > > > > > Yes, I have mistaken the mask, and yes, it is actually not necessary :) > Also thanks for removing the duplicate code, actually I followed the pattern > of my code and there I log the type of frame for which the session was not > found, hence the duplication here. So you can commit the patch. > > Thank you! > Alex > |
|
From: Alexander V. <ava...@vo...> - 2008-04-23 16:38:57
Attachments:
avasilev.vcf
|
You are welcome, Pete :) Thank you for the fast response. Greets Alex Peter Grayson wrote: > Hi Alex, > > Thank you for your help with this patch. I have committed the changes to trunk. > > I will be merging this change along with several others into the 2.1 > branch in the near future. > > Pete > > On Wed, Apr 23, 2008 at 11:06 AM, Alexander Vassilev > <ava...@vo...> wrote: > >> Hello Pete, >> >> >> >>> Please let me know if you think this updated patch is okay. >>> >>> >>> >> Yes, I have mistaken the mask, and yes, it is actually not necessary :) >> Also thanks for removing the duplicate code, actually I followed the pattern >> of my code and there I log the type of frame for which the session was not >> found, hence the duplication here. So you can commit the patch. >> >> Thank you! >> Alex >> >> > > |
×
Want the latest updates on software, tech news, and AI?
Get latest updates about software, tech news, and AI from SourceForge directly in your inbox once a month.
Thanks for helping keep SourceForge clean.
X