Re: Disable printing bug?

[Iain Morgan]

I haven't seen anyone address this issue. The problem is due to using
readlink() without first initializing the buffer.  I have found a few
other instances where this problem could occur.

The attached patch rectifies these issues. It also sets the length of the
buffer to bufsize - 1 when calling readlink.  This is to ensure that the
resulting string is properly terminated. (Remember that readlink() does not
null-terminate the string that it places in the buffer.)

There is one call to cfreadlink() in image.c that may still require 
initialization of the buffer.  It might actually be a good idea to
do the initialization of the buffer inside of cfreadlink() to avoid any
future issues.

I have not checked if this patch interferes with any patches that have been
released after 1.6.2. Also I have not done any rigorous testing.  The only
testing that I have done so far is to confirm that cfengine compiles
correctly with the patch.

On Thu Jan 25 17:34:45 2001, Robert Shaw wrote:
> 
> Mark,
> 
> After upgrading to 1.6.2 (at least I think it occurs only with the 
> new version, not verified though). If I have a "disable:" action 
> setup for an item, and it goes to delete a link, the message looks a 
> little messed up:
> 
> cf:arizona5: Deleting link /usr/bin/perl -> /usr/local/bin/perle
> cf:arizona5: Deleting link /usr/bin/python -> /usr/local/bin/pythondx
> 
> Notice the trailing characters. They don't really exist in what the 
> link was pointing to. This looks like a cosmetic problem only, but I 
> can't be sure if it will affect anything else.
> 
> Can you please look at it?
> 
> Thanks.
> -Robert
> -- 
> -Robert
>   mailto:address@hidden
>   http://members.home.net/rshaw-az/
>   AIM: RShawPPC
> 
> _______________________________________________
> Bug-cfengine mailing list
> address@hidden
> http://mail.gnu.org/mailman/listinfo/bug-cfengine
> 

diff -uar cfengine-1.6.2.orig/src/cfd.c cfengine-1.6.2/src/cfd.c
--- cfengine-1.6.2.orig/src/cfd.c       Sun Jan 14 11:25:09 2001
+++ cfengine-1.6.2/src/cfd.c    Wed Feb 14 12:34:30 2001
@@ -1967,7 +1967,7 @@
    cfst.cf_lmode = statbuf.st_mode & 07777;
    cfst.cf_nlink = statbuf.st_nlink;
        
-   if (readlink(filename,linkbuf,bufsize) == -1)
+   if (readlink(filename,linkbuf,bufsize - 1) == -1)
       {
       sprintf(sendbuffer,"BAD: unable to read link\n");
       CfLog(cferror,sendbuffer,"readlink");
diff -uar cfengine-1.6.2.orig/src/do.c cfengine-1.6.2/src/do.c
--- cfengine-1.6.2.orig/src/do.c        Thu Nov 30 15:06:46 2000
+++ cfengine-1.6.2/src/do.c     Wed Feb 14 12:36:50 2001
@@ -1455,7 +1455,8 @@
          continue;
          }
 
-      if (readlink(workname,VBUFF,bufsize) == -1)
+      bzero (VBUFF, bufsize);
+      if (readlink(workname,VBUFF,bufsize - 1) == -1)
          {
          sprintf(OUTPUT,"DisableFiles() can't read link %s\n",workname);
         CfLog(cferror,OUTPUT,"readlink");
diff -uar cfengine-1.6.2.orig/src/edittools.c cfengine-1.6.2/src/edittools.c
--- cfengine-1.6.2.orig/src/edittools.c Mon Jan  1 09:38:32 2001
+++ cfengine-1.6.2/src/edittools.c      Wed Feb 14 12:39:26 2001
@@ -254,7 +254,7 @@
       bzero(linkname,bufsize);
       bzero(realname,bufsize);
       
-      if (readlink(filename,linkname,bufsize) == -1)
+      if (readlink(filename,linkname,bufsize - 1) == -1)
         {
         sprintf(OUTPUT,"Cannot read link %s\n",filename);
         CfLog(cferror,OUTPUT,"readlink");
diff -uar cfengine-1.6.2.orig/src/filters.c cfengine-1.6.2/src/filters.c
--- cfengine-1.6.2.orig/src/filters.c   Sun Dec 10 10:36:04 2000
+++ cfengine-1.6.2/src/filters.c        Wed Feb 14 12:44:12 2001
@@ -1291,7 +1291,8 @@
    return false;
    }
  
-if (readlink(filename,buffer,bufsize) == -1)
+bzero (buffer, bufsize);
+if (readlink(filename,buffer,bufsize - 1) == -1)
    {
    sprintf(OUTPUT,"Unable to read link %s in filter",filename);
    CfLog(cferror,OUTPUT,"readlink");
diff -uar cfengine-1.6.2.orig/src/image.c cfengine-1.6.2/src/image.c
--- cfengine-1.6.2.orig/src/image.c     Fri Jan  5 13:27:53 2001
+++ cfengine-1.6.2/src/image.c  Wed Feb 14 13:02:26 2001
@@ -1046,7 +1046,7 @@
 
 if (strcmp(ip->server,"localhost") == 0)
    {
-   return readlink(sourcefile,linkbuf,buffsize);
+   return readlink(sourcefile,linkbuf,buffsize - 1);
    }
 
 for (sp = ip->cache; sp != NULL; sp=sp->next)
diff -uar cfengine-1.6.2.orig/src/link.c cfengine-1.6.2/src/link.c
--- cfengine-1.6.2.orig/src/link.c      Wed Oct 18 05:28:14 2000
+++ cfengine-1.6.2/src/link.c   Wed Feb 14 13:20:00 2001
@@ -355,7 +355,7 @@
         CfLog(cfverbose,OUTPUT,"");
         
         bzero(VBUFF,bufsize);
-         if (readlink(newto,VBUFF,bufsize) != -1)
+         if (readlink(newto,VBUFF,bufsize - 1) != -1)
             {
             Verbose("File is link to -> %s\n",VBUFF);
             }
@@ -565,7 +565,7 @@
 
 bzero(linkbuf,bufsize);
 
-if (readlink(from,linkbuf,bufsize) == -1)
+if (readlink(from,linkbuf,bufsize - 1) == -1)
    {
    if (! MakeDirectoriesFor(from))                  /* link doesn't exist */
       {
@@ -821,7 +821,7 @@
 bzero(linkbuf,bufsize);
 bzero(linkpath,bufsize); 
 
-if (readlink(name,linkbuf,bufsize) == -1)
+if (readlink(name,linkbuf,bufsize - 1) == -1)
    {
    sprintf(OUTPUT,"(Can't read link %s while checking for deadlinks)\n",name);
    CfLog(cfverbose,OUTPUT,"");
@@ -1089,7 +1089,7 @@
       {
       bzero(buff,bufsize);
       
-      if (readlink(dest,buff,bufsize) == -1)
+      if (readlink(dest,buff,bufsize - 1) == -1)
         {
         sprintf(OUTPUT,"Expand links can't stat %s\n",dest);
         CfLog(cferror,OUTPUT,"readlink");
diff -uar cfengine-1.6.2.orig/src/tidy.c cfengine-1.6.2/src/tidy.c
--- cfengine-1.6.2.orig/src/tidy.c      Tue Nov 21 13:20:49 2000
+++ cfengine-1.6.2/src/tidy.c   Wed Feb 14 13:12:33 2001
@@ -137,7 +137,8 @@
             {
             sprintf(OUTPUT,"Can't stat %s\n",pcwd);
            CfLog(cferror,OUTPUT,"lstat");
-            if (readlink(pcwd,VBUFF,bufsize) != -1)
+           bzero (VBUFF, sizeof (VBUFF));
+            if (readlink(pcwd,VBUFF,bufsize - 1) != -1)
                {
                sprintf(OUTPUT,"File is link to -> %s\n",VBUFF);
               CfLog(cferror,OUTPUT,"");
@@ -283,7 +284,8 @@
         sprintf(OUTPUT,"Can't stat %s\n",name);
         CfLog(cferror,OUTPUT,"");
         
-        if (readlink(name,VBUFF,bufsize) != -1)
+        bzero (VBUFF, sizeof (VBUFF));
+        if (readlink(name,VBUFF,bufsize - 1) != -1)
            {
            sprintf(OUTPUT,"File is link to -> %s\n",VBUFF);
            CfLog(cferror,OUTPUT,"");
@@ -346,7 +348,8 @@
             {
             sprintf(OUTPUT,"Can't stat %s\n",pcwd);
            CfLog(cferror,OUTPUT,"lstat");
-            if (readlink(pcwd,VBUFF,bufsize) != -1)
+           bzero (VBUFF, sizeof (VBUFF));
+            if (readlink(pcwd,VBUFF,bufsize - 1) != -1)
                {
                sprintf(OUTPUT,"File is link to -> %s\n",VBUFF);
               CfLog(cferror,OUTPUT,"");

--
Iain Morgan
NAS Desktop Support Group