[nas] nas: Multiple Vulnerabilities in nas 1.9.3

[Jon Trulson]

On Tue, 13 Aug 2013, Erik Auerswald wrote:

> Hi,
>
> On 08/13/2013 08:45 PM, Jon Trulson wrote:
>> On Tue, 13 Aug 2013, Erik Auerswald wrote:
>>> 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
[...]
>>> How about the following patch?
>> 
>> I think this patch should be fine.  If someone can play a symlink
>> attack here, then they already have root access and all is lost
>> already :)
>
> I don't really like my first patch and suggest the following (same idea, 
> different realization).
>
> ---8<---
> Index: server/os/connection.c
> ===================================================================
> --- server/os/connection.c	(revision 288)
> +++ server/os/connection.c	(working copy)
> @@ -421,10 +421,16 @@
>
>     /* Allow the audio server to run on a different IP device with the
>      * TCP_DEVICE environment variable, otherwise we take the default.
> +     * A different IP device is specified via its number concatenated
> +     * to the TCP device name, so check that the default TCP device is
> +     * a prefix of the value read from the TCP_DEVICE environment
> +     * variable.
>      */
>     tcp_dev = getenv("TCP_DEVICE");
> -    if (tcp_dev == NULL)
> +    if (!tcp_dev || strncmp(tcp_dev, TCP_DEVICE, strlen(TCP_DEVICE))) {
> +        fprintf(stderr, "Ignoring invalid TCP_DEVICE environment 
> variable.\n");
>         tcp_dev = TCP_DEVICE;
> +    }
>
>     fd = open(tcp_dev, O_RDWR);
>     if (fd == -1) {
> ---8<---
>
> [Patch looks white space damaged (fprintf() line) in my mail client.]
>
> Please double-check the if condition, as str[n]cmp can be quite confusing. 
> ;-)
>

This looks fine.

> Using osLogMsg() instead of fprintf() might be better, but I did not want to 
> introduce this to connection.c without a Minix system to test the change.
>

osLogMsg() would be better (in case started as part of boot up), but
yes - I do not have minix installed anywhere either so I could not
test this (ie: does osLogMsg() even work properly under minix?).

Wonder if we should just drop minix support some day.


-- 
Jon Trulson

   "I was not genomed to alter reality."
       - Sonmi 451