[asterisk-commits] mjordan: branch 1.8 r367843 - /branches/1.8/channels/chan_skinny.c

[SVN commits to the Asterisk project]

Author: mjordan
Date: Tue May 29 13:30:25 2012
New Revision: 367843

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=367843
Log:
AST-2012-008: Fix remote crash vulnerability in chan_skinny

When a skinny session is unregistered, the corresponding device pointer is set
to NULL in the channel private data.  If the client was not in the on-hook state
at the time the connection was closed, the device pointer can later be
dereferenced if a message or channel event attempts to use a line's pointer to
said device.

The patches prevent this from occurring by checking the line's pointer in
message handlers and channel callbacks that can fire after an unregistration
attempt.

(closes issue ASTERISK-19905)
Reported by: Christoph Hebeisen
Tested by: mjordan, Damien Wedhorn
Patches:
  AST-2012-008-1.8.diff uploaded by mjordan (license 6283)
  AST-2012-008-10.diff uploaded by mjordan (license 6283)


Modified:
    branches/1.8/channels/chan_skinny.c

Modified: branches/1.8/channels/chan_skinny.c
URL: http://svnview.digium.com/svn/asterisk/branches/1.8/channels/chan_skinny.c?view=diff&rev=367843&r1=367842&r2=367843
==============================================================================
--- branches/1.8/channels/chan_skinny.c (original)
+++ branches/1.8/channels/chan_skinny.c Tue May 29 13:30:25 2012
@@ -2677,6 +2677,10 @@
 	struct skinny_line *l = sub->parent;
 	struct skinny_device *d = l->device;
 
+	if (!d) {
+		return;
+	}
+
 	if (!c->caller.id.number.valid
 		|| ast_strlen_zero(c->caller.id.number.str)
 		|| !c->connected.id.number.valid
@@ -3810,6 +3814,11 @@
 	int res = 0;
 	int loop_pause = 100;
 
+	if (!d) {
+		ast_log(LOG_WARNING, "Device for line %s is not registered.\n", l->name);
+		return NULL;
+	}
+
 	ast_verb(3, "Starting simple switch on '%s@%s'\n", l->name, d->name);
 
 	len = strlen(d->exten);
@@ -3913,7 +3922,7 @@
 	struct skinny_line *l = sub->parent;
 	struct skinny_device *d = l->device;
 
-	if (!d->registered) {
+	if (!d || !d->registered) {
 		ast_log(LOG_ERROR, "Device not registered, cannot call %s\n", dest);
 		return -1;
 	}
@@ -3976,6 +3985,11 @@
 
 	l = sub->parent;
 	d = l->device;
+
+	if (!d) {
+		ast_log(LOG_WARNING, "Device for line %s is not registered.\n", l->name);
+		return 0;
+	}
 
 	if (skinnydebug)
 		ast_verb(3,"Hanging up %s/%d\n",d->name,sub->callid);
@@ -4374,7 +4388,13 @@
 	struct skinny_subchannel *sub = ast->tech_pvt;
 	struct skinny_line *l = sub->parent;
 	struct skinny_device *d = l->device;
-	struct skinnysession *s = d->session;
+	struct skinnysession *s;
+
+	if (!d) {
+		ast_log(LOG_WARNING, "Device for line %s is not registered.\n", l->name);
+		return -1;
+	}
+	s = d->session;
 
 	if (!s) {
 		ast_log(LOG_NOTICE, "Asked to indicate '%s' condition on channel %s, but session does not exist.\n", control2str(ind), ast->name);
@@ -4608,8 +4628,13 @@
 	struct skinny_device *d = l->device;
 
 	/* Don't try to hold a channel that doesn't exist */
-	if (!sub || !sub->owner)
+	if (!sub || !sub->owner) {
 		return 0;
+	}
+	if (!d) {
+		ast_log(LOG_WARNING, "Device for line %s is not registered.\n", l->name);
+		return 0;
+	}
 
 	/* Channel needs to be put on hold */
 	if (skinnydebug)
@@ -4635,8 +4660,13 @@
 	struct skinny_device *d = l->device;
 
 	/* Don't try to unhold a channel that doesn't exist */
-	if (!sub || !sub->owner)
+	if (!sub || !sub->owner) {
 		return 0;
+	}
+	if (!d) {
+		ast_log(LOG_WARNING, "Device for line %s is not registered.\n", l->name);
+		return 0;
+	}
 
 	/* Channel is on hold, so we will unhold */
 	if (skinnydebug)
@@ -4689,6 +4719,11 @@
 
 	l = sub->parent;
 	d = l->device;
+
+	if (!d) {
+		ast_log(LOG_WARNING, "Device for line %s is not registered.\n", l->name);
+		return -1;
+	}
 
 	if (!sub->related) {
 		/* Another sub has not been created so this must be first XFER press */
@@ -4820,6 +4855,11 @@
 	struct skinny_device *d = l->device;
 	struct ast_channel *c = sub->owner;
 	pthread_t t;
+
+	if (!d) {
+		ast_log(LOG_WARNING, "Device for line %s is not registered.\n", l->name);
+		return 0;
+	}
 
 	if (l->hookstate == SKINNY_ONHOOK) {
 		l->hookstate = SKINNY_OFFHOOK;