Summary:	ASTERISK-07566: chan_skinny interger overflow (dos/possible remote compromise)
Reporter:	Anthony LaMantia (alamantia)	Labels:
Date Opened:	2006-08-20 17:36:11	Date Closed:	2006-08-21 02:35:57
Priority:	Major	Regression?	No
Status:	Closed/Complete	Components:	Channels/chan_agent
Versions:		Frequency of Occurrence
Related Issues:
Environment:		Attachments:	( 0) skinny_patch.diff
Description:	there is an interger overflow inside the get_input function inside of chan_skinny that allows anyone client which has established a active tcp connection to the skinny channel the ability to crash asterisk as well as comprimise a few cirtical structures within the channel driver. the patched posted should repair the issue. -Anthony
Comments:	By: Jason Parker (jparker) 2006-08-20 23:14:14 It may just be that I'm tired, but I don't quite understand the intentions of this patch.  The read() that sets dlen reads exactly 4 bytes from the stream, and converts it to an int.  If almost looks like you're trying to protect from what I would call an "underflow". Can you explain a little more? Also, all patches need to be in unified format.  diff -u, or svn diff By: Anthony LaMantia (alamantia) 2006-08-21 00:52:11 it's quite simple. 03906 static int get_input(struct skinnysession *s) 03907 { 03908    int res; 03909    int dlen = 0; 03910    struct pollfd fds[1]; 03911 dlen is a signed 32bit integer and inbuf is a 1,000 byte char array located inside of a skinnysession struct. 03927       res = read(s->fd, s->inbuf, 4); when any tcp connection is made to port 2000 get_input will read 4 bytes in to inbuf dlen = letohl(*(int *)s->inbuf); dlen is set to the assigned contents of the 32bits sent over the network to s->inbuf consider for this example the remote client sends this send(sd,"\xfb\xff\xff\xff",4,0); the value of dlen in memory as a signed integer would be -5 then.. 03936       if (dlen+8 > sizeof(s->inbuf)) { 03937          dlen = sizeof(s->inbuf) - 8; 03938       } 03939       *(int *)s->inbuf = htolel(dlen); the above math is all preformed treating dlen as a signed integer -5 + 8 = 3  and thats not larger then 1000..so dlen keeps it's retains.. after this the vulnerablity is exposed with this read 03941       res = read(s->fd, s->inbuf+4, dlen+4); ssize_t read(int fd, void *buf, size_t count); count, us an unsigned 16bit number (the max being 65535) 0xfffffffb + 0x4 = 0xffffffff thus read will attempt to read into the 1,000 byte buffer 65535 bytes which would make it possible  for an attacker to  control importent bits of memory inside of the asterisk process... so... if an attacker sends chan_skinny a 4byte packet that contains \xfb\xff\xff\xff with one send() followed directly by  4,000 A's (for example) in another send() asterisk will crash via chan_skinny   see destory_session(s); that is called after get_input()  inside of skinny_session.. this may all be sort of hard to read, and im a bit overtired tommrow i can post another patch, and an example file that will cash asterisk when running chan_skinny good night -anthony By: Anthony LaMantia (alamantia) 2006-08-21 00:54:42 oh also, the bug should really be classified as a signedness issue rather then an intergeroverflow, sorry about that.. i usually just class all of these sorts of problems as interger overflows.. By: Jason Parker (jparker) 2006-08-21 01:17:06 Made this bug private for now - for obvious reasons By: Jason Parker (jparker) 2006-08-21 02:35:57 Fix committed in revision 40757.