[nas] Improper fixes in the r288

[Petr Pisar]

Hello,

I read some patches for the `Multiple Vulnerabilities in nas 1.9.3' issue and
I think some changes are not sufficient.

E.g. here:

--- server/os/access.c  (revision 287)
+++ server/os/access.c  (revision 288)
@@ -478,9 +478,9 @@
         validhosts = host->next;
         FreeHost(host);
     }
-    strcpy(fname, "/etc/X");
-    strcat(fname, display);
-    strcat(fname, ".hosts");
+    strncpy(fname, "/etc/X", sizeof fname); fname[sizeof fname - 1] = '\0';
+    strncat(fname, display, sizeof fname - strlen(fname) - 1);
+    strncat(fname, ".hosts", sizeof fname - strlen(fname) - 1);
     if (fd = fopen(fname, "r")) {
         while (fgets(hostname, sizeof(hostname), fd)) {
             if (ptr = index(hostname, '\n'))

the `sizeof fname - strlen(fname) - 1' will result into -1 if previous command
fills fname completely. And because the last argument is of type size_t that
is unsigned, you will pass a very high positive number.

Moreover if the copied text gets clamped, you won't notice it and you will
try to open wrong file in turn.

In this case, it would be much more elligible to use snprintf() and check the
return value:

  int r = snprinf(fname, sizeof fname, "/etc/X%s.hosts", display);
  if (r < 0 || r >= sizeof fname))
    FatalError();


Or here:

--- server/os/connection.c      (revision 287)
+++ server/os/connection.c      (revision 288)
@@ -2115,7 +2127,7 @@
         if (AuServerHostName == NULL)
             FatalError
                     ("AUDIOHOST not set, or server host name not given\n");
-        sprintf(host, "%s/%s:%s", DEF_AUSVRDIR, AuServerHostName,
+        snprintf(host, sizeof host, "%s/%s:%s", DEF_AUSVRDIR, AuServerHostName,
                 0 /* port */ );
 
         uniqport(&Au.cap_port);

the host will be undefined, if snprintf() cannot fit the string into the its
size. You should check for sprintf clamp again.

-- Petr
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 230 bytes
Desc: not available
URL: <http://radscan.com/pipermail/nas/attachments/20130912/d200bb78/attachment.pgp>