zaptel: insufficient input validation in some zaptel drivers

[Tzafrir Cohen]

Hi

I wanted to let you know of an issue I spotted that may impact some
Debian systems.

Package: zaptel
Version: all versions (Now fixed in SVN, rev 4588)
Upstream issue: http://bugs.digium.com/view.php?id=13954
Impact: local privileges escalation

Fix for Etch version: attached dpatch
Fix for Lenny version: http://svn.debian.org/viewsvn/pkg-voip?rev=6507&view=rev

Impact: Some older Zaptel drivers do not apply input validation on the
sync field from the ioctl ZT_SPANCONFIG . This is sent on /dev/zap/ctl ,
which in Debian is writable to the group dialout.

In Zaptel this ioctl is mostly handled by the specific spanconfig
function of the low-level driver. Thus this will not have any impact
unless someone has the matching hardware. 

* torisa.c is a driver for the old ISA dual-span T1 card, which I believe
  nobody actually uses. 
* tor2.c is the driver for the Zapata Telephony Tormenta 2 card quad
  T1/E1 card. Still sold today.

Those two drivers use one specific field from the ioctl struct as an
array index and write there, assuming it is between 0 and 1 (torisa) or
3 (tor2). So we have a nice way to write over many places in kernel
space. The value to write, though, is not easy to control and can't even
be 0.

* wct1xxp.c is the driver for Digium's earlier single-span E1 cards
  (now deprecated: E100P and T100P.
* wcte11xp is the driver for Digium's TE110P, which was was slightly
  better, but replaced is now EOL.

The issue with those two is that the value from this field is written to
a register, while we only wanted to get its first bit. I'm still not
sure if it has any interesting impact for the user, but it is definetly
misbehaving.

Analog cards do not have a spanconfig method. Our package also includes
several other drivers for digital cards that do have a spanconfig method
(cwain, qozap, zaphfc, vzaphfc and ztgsm in Etch, and in Lenny: also
ds1x1f) but none of them seem to have this problem.

-- 
               Tzafrir Cohen
icq#16849755              jabber:tzafrir.cohen at xorcom.com
+972-50-7952406           mailto:tzafrir.cohen at xorcom.com
http://www.xorcom.com  iax:guest at local.xorcom.com/tzafrir
-------------- next part --------------
#! /bin/sh /usr/share/dpatch/dpatch-run
## fix_sync_validation.dpatch by Tzafrir Cohen <tzafrir.cohen at xorcom.com>
##
## All lines beginning with `## DP:' are a description of the patch.
## DP: Don't trust input coming from the field 'sync' in the ioctl 
## DP: ZT_SPANCONFIG (e.g. the span "timing" in zaptel.conf)
## DP:
## DP: Upstream issue: http://bugs.digium.com/view.php?id=13954 

@DPATCH@
Index: 1.2/wcte11xp.c
===================================================================
--- 1.2/wcte11xp.c	(revision 4586)
+++ 1.2/wcte11xp.c	(revision 4587)
@@ -932,7 +932,7 @@
 	span->txlevel = lc->lbo;
 	span->rxlevel = 0;
 	/* Do we want to SYNC on receive or not */
-	wc->sync = lc->sync;
+	wc->sync = (lc->sync) ? 1 : 0;
 	/* If already running, apply changes immediately */
 	if (span->flags & ZT_FLAG_RUNNING)
 		return t1xxp_startup(span);
Index: 1.2/tor2.c
===================================================================
--- 1.2/tor2.c	(revision 4586)
+++ 1.2/tor2.c	(revision 4587)
@@ -203,6 +203,13 @@
 
 	if (debug)
 		printk("Tor2: Configuring span %d\n", span->spanno);
+
+	if ((lc->sync < 0) || (lc->sync >= SPANS_PER_CARD)) {
+		printk(KERN_WARNING "%s %d: invalid span timing value %d.\n",
+				THIS_MODULE->name, span->spanno, lc->sync);
+		return -EINVAL;
+	}
+
 	/* XXX We assume lineconfig is okay and shouldn't XXX */	
 	span->lineconfig = lc->lineconfig;
 	span->txlevel = lc->lbo;
Index: 1.2/torisa.c
===================================================================
--- 1.2/torisa.c	(revision 4586)
+++ 1.2/torisa.c	(revision 4587)
@@ -602,6 +602,13 @@
 {
 	if (debug)
 		printk("TorISA: Configuring span %d\n", span->spanno);
+
+	if ((lc->sync < 0) || (lc->sync >= 2)) {
+		printk(KERN_WARNING "%s %d: invalid span timing value %d.\n",
+				THIS_MODULE->name, span->spanno, lc->sync);
+		return -EINVAL;
+	}
+
 	/* XXX We assume lineconfig is okay and shouldn't XXX */	
 	span->lineconfig = lc->lineconfig;
 	span->txlevel = lc->lbo;
Index: 1.2/wct1xxp.c
===================================================================
--- 1.2/wct1xxp.c	(revision 4586)
+++ 1.2/wct1xxp.c	(revision 4587)
@@ -738,7 +738,7 @@
 	span->txlevel = lc->lbo;
 	span->rxlevel = 0;
 	/* Do we want to SYNC on receive or not */
-	wc->sync = lc->sync;
+	wc->sync = (lc->sync) ? 1 : 0;
 	/* If already running, apply changes immediately */
 	if (span->flags & ZT_FLAG_RUNNING)
 		return t1xxp_startup(span);