Use FD rather than filename to check configuration file security. - susmb - mounting of SMB/CIFS shares via FUSE
 (HTM) git clone git://git.codemadness.org/susmb
 (DIR) Log
 (DIR) Files
 (DIR) Refs
 (DIR) README
 (DIR) LICENSE
       ---
 (DIR) commit b32599fdd33a7b726236a661bad851cffbbbd950
 (DIR) parent 647c2a94ac16b087c5423256ca45aba0a0eb7d43
 (HTM) Author: Geoff Johnstone <geoffSHEEP.johnstoneFROG@googlemail.com>
       Date:   Fri, 30 May 2008 19:12:02 +0100
       
       Use FD rather than filename to check configuration file security.
       
       Diffstat:
         M conffile.c                          |      54 ++++++++++++++++++++++++++++++-
         M usmb.c                              |      38 +------------------------------
       
       2 files changed, 54 insertions(+), 38 deletions(-)
       ---
 (DIR) diff --git a/conffile.c b/conffile.c
       @@ -15,20 +15,72 @@
         */
        
        #include <assert.h>
       +#include <errno.h>
        #include <stdio.h>
        #include <stdlib.h>
        #include <string.h>
       +#include <sys/types.h>
       +#include <sys/stat.h>
       +#include <fcntl.h>
       +#include <unistd.h>
        #include "password.h"
        #include "utils.h"
        #include "xml.h"
        #include "config.rng.h"
        
        
       +static bool check_conf_perms (const char *conffile, int fd)
       +{
       +  struct stat buf;
       +
       +  if (0 == fstat (fd, &buf))
       +  {
       +    if (getuid() != buf.st_uid)
       +    {
       +      fprintf (stderr, "You do not own the configuration file %s\n", conffile);
       +      return false;
       +    }
       +
       +    if (buf.st_mode & (S_IRWXG | S_IRWXO))
       +    {
       +      fprintf (stderr, "Configuration file %s is accessible to non-owner\n",
       +               conffile);
       +      return false;
       +    }
       +  }
       +
       +  else
       +  {
       +    fprintf (stderr, "Cannot stat configuration file %s: %s\n",
       +             conffile, strerror (errno));
       +    return false;
       +  }
       +
       +  return true;
       +}
       +
       +
        static bool conffile_read (const char *filename,
                                   xmlDocPtr *doc,
                                   xmlXPathContextPtr *ctx)
        {
       -  *doc = xmlParseFile (filename);
       +  int fd = open (filename, O_RDONLY);
       +  if (-1 == fd)
       +  {
       +    fprintf (stderr, "Cannot open %s: %s\n", filename, strerror (errno));
       +    return false;
       +  }
       +
       +  if (!check_conf_perms (filename, fd))
       +  {
       +    (void)close (fd);
       +    return false;
       +  }
       +
       +  //*doc = xmlParseFile (filename);
       +  *doc = xmlReadFd (fd, NULL, NULL, XML_PARSE_NONET);
       +  (void)close (fd);
       +
          if (NULL == *doc)
          {
            fprintf (stderr, "Cannot parse %s\n", filename);
 (DIR) diff --git a/usmb.c b/usmb.c
       @@ -15,13 +15,10 @@
         */
        
        #include <sys/time.h>        // struct timeval needed by libsmbclient.h
       -#include <sys/types.h>
       -#include <sys/stat.h>
        #include <unistd.h>
        #include <libsmbclient.h>
        #include <fuse.h>
        #include <assert.h>
       -#include <errno.h>
        #include <stdarg.h>
        #include <stdbool.h>
        #include <stddef.h>
       @@ -189,38 +186,6 @@ static struct fuse_operations fuse_ops = {
        };
        
        
       -// this should really open() the file and check the fd, but the XML parser
       -// takes a filename, not a file descriptor
       -static bool check_conf_perms (const char *conffile)
       -{
       -  struct stat buf;
       -  if (0 == stat (conffile, &buf))
       -  {
       -    if (getuid() != buf.st_uid)
       -    {
       -      fprintf (stderr, "You do not own the configuration file %s\n",
       -               conffile);
       -      return false;
       -    }
       -
       -    if (buf.st_mode & (S_IRWXG | S_IRWXO))
       -    {
       -      fprintf (stderr, "Configuration file %s is accessible to non-owner\n",
       -               conffile);
       -      return false;
       -    }
       -  }
       -  else
       -  {
       -    fprintf (stderr, "Cannot stat configuration file %s: %s\n",
       -             conffile, strerror (errno));
       -    return false;
       -  }
       -
       -  return true;
       -}
       -
       -
        static bool create_share_name (const char *server, const char *sharename)
        {
          size_t len = strlen ("smb:///") +
       @@ -274,8 +239,7 @@ int main (int argc, char **argv)
        
          show_about (stdout);
        
       -  if (!check_conf_perms (conffile) ||
       -      !conffile_get_mount (conffile, mountid,
       +  if (!conffile_get_mount (conffile, mountid,
                                   &server, &sharename, &mountpoint, &options,
                                   &domain, &username, &password))
            return EXIT_FAILURE;