[nas] nas: Multiple Vulnerabilities in nas 1.9.3

[Erik Auerswald]

Hi,

On Tue, Aug 13, 2013 at 04:07:33PM +0200, Erik Auerswald wrote:
> On Mon, Aug 12, 2013 at 11:28:50AM -0600, Jon Trulson wrote:
> > On Mon, 12 Aug 2013, Erik Auerswald wrote:
> >> On Sun, Aug 11, 2013 at 10:09:13AM +0430, Hamid Zamani wrote:
> >>> On 08/10/2013 10:08 PM, Erik Auerswald wrote:
> >>>> I do not know how to determine if a given string is a valid or invalid TCP
> >>>> device on Minix.
> >>>
> >>> on Minix i checked the /dev/tcp :
> >>>
> >>> # ls -la /dev/tcp*
> >>> crw-rw-rw- ... /dev/tcp
> >>> crw-rw-rw- ... /dev/tcp0
> >>>
> >>> So checking validation can be done as checking the string to be a `valid
> >>> Character special file`.
> >>>
> >>> i think just using a stat struct does the trick. ;)
> >>>
> >>> ===
> >>>      if (status.st_mode & S_IFCHR)
> >>>          do so ...
> >>> ===
> >>>
> >>> of course a symlink may be used to attack (a little stricture ;-) )but i
> >>> think it is better choice than leaving it.
> >>
> >> I have searched for Minix docs and it seems to me that the TCP device name
> >> should always start with /dev/tcp. Using a numbered TCP device chooses the
> >> respective network interface, no number chooses the default network
> >> interface.
> >>
> >> I would suggest to check if the string in the environment variable starts
> >> with /dev/tcp. WDYT?
> >>
> >> Should we check that the device name does not contain any /../ parts
> >> as well? Possibly reject anything but digits after the initial /dev/tcp?
> >
> > Could you just check for a number instead?  Ie: Instead of TCP_DEVICE,
> > use something like TCP_DEVNUM, which if set, is expected to be some
> > (smallish) integer that can be tacked onto /dev/tcp in the code...?
> 
> Do you have any references to TCP_DEVNUM? Quick googling did not turn
> up anything. I suppose that would be new, special environment variable
> just for nasd on Minix.
> 
> I do not like to change from the usual TCP_DEVICE (see
> http://www.minix3.org/manpages/html4/ip.html) to something
> special just for NAS. I do not know if anybody at all uses NAS on Minix and
> I don't even compile the code in question (because of #ifdef).
> 
> My research so far hints at TCP/IP devices having major 7
> (http://www.minix-vmd.org/pub/Minix-vmd/1.7.0/cat/cat4/dev.4), which could
> be checked as well if it still holds.
> 
> Currently, Minix seems to be restricted to a maximum of 16 network devices,
> so /dev/tcp{,{0..15}} should describe the legal values for TCP_DEVICE.
> (see http://www.minix3.org/manpages/html8/inet.html)
> 
> Anyway, the first the current code tries to do with the opened TCP_DEVICE
> is to an ioctl() call. If that fails, the file descriptor is closed. As
> long as ioctl() is not exploitable, this does not constitute a
> vulnerability.
> 
> A sanity check of program input, in this case from an environment variable,
> is prudent. Verifying a prefix of "/dev/tcp" would go a long way.

How about the following patch?

---8<---
Index: server/os/connection.c
===================================================================
--- server/os/connection.c      (revision 288)
+++ server/os/connection.c      (working copy)
@@ -421,9 +421,11 @@
 
     /* Allow the audio server to run on a different IP device with the 
      * TCP_DEVICE environment variable, otherwise we take the default.
+     * Valid TCP devices on Minix are /dev/tcp[NUMBER]. The NUMBER range
+     * depends on Minix version.
      */
     tcp_dev = getenv("TCP_DEVICE");
-    if (tcp_dev == NULL)
+    if (tcp_dev == NULL || (strncmp(tcp_device, "/dev/tcp", 8) != 0))
         tcp_dev = TCP_DEVICE;
 
     fd = open(tcp_dev, O_RDWR);
---8<---

Thanks,
Erik