Bugfix in photoid.c and keyserver.c

Properly handle permission/ownership checks on files that are shared (for
example /usr/local/lib/gnupg/idea)

8 files changed, 73 insertions, 28 deletions

diff --git a/g10/ChangeLog b/g10/ChangeLog
index 593b415c3..339653e22 100644
--- a/g10/ChangeLog
+++ b/g10/ChangeLog
@@ -1,5 +1,17 @@
 2001-12-28  David Shaw  <[email protected]>
 
+	* g10.c (main): Use a different strlist to check extensions since
+	they need to be handled seperately now.
+
+	* misc.c,main.h (check_permissions): Properly handle permission
+	and ownership checks on files in the lib directory
+	(e.g. /usr/local/lib/gnupg), which are owned by root and are
+	world-readable, and change all callers to specify extension or
+	per-user file.
+
+	* photoid.c (show_photo), keyserver.c (keyserver_spawn): Bug fix -
+	don't call exec_finish if exec_write fails.
+
 	* keyserver.c (keyserver_spawn): Look for OPTIONS from the
 	keyserver helper - specifically, a "OUTOFBAND" option for the
 	email keyserver.
diff --git a/g10/g10.c b/g10/g10.c
index 1fdc99c59..c3fbefbbc 100644
--- a/g10/g10.c
+++ b/g10/g10.c
@@ -695,6 +695,7 @@ main( int argc, char **argv )
     const char *fname;
     char *username;
     STRLIST unsafe_files=NULL;
+    STRLIST extensions=NULL;
     int may_coredump;
     STRLIST sl, remusr= NULL, locusr=NULL;
     STRLIST nrings=NULL, sec_nrings=NULL;
@@ -829,7 +830,7 @@ main( int argc, char **argv )
   next_pass:
     if( configname ) {
 
-      if(check_permissions(configname,1))
+      if(check_permissions(configname,0,1))
 	{
 	  add_to_strlist(&unsafe_files,configname);
 
@@ -1017,8 +1018,7 @@ main( int argc, char **argv )
 	  case oAlwaysTrust: opt.always_trust = 1; break;
 	  case oLoadExtension:
 #ifndef __riscos__
-	    if(check_permissions(pargs.r.ret_str,1))
-	      add_to_strlist(&unsafe_files,pargs.r.ret_str);
+	    add_to_strlist(&extensions,pargs.r.ret_str);
 	    register_cipher_extension(orig_argc? *orig_argv:NULL,
 				      pargs.r.ret_str);
 #else /* __riscos__ */
@@ -1223,18 +1223,28 @@ main( int argc, char **argv )
     }
   #endif
 
-    check_permissions(opt.homedir,0);
+    check_permissions(opt.homedir,0,0);
 
     if(unsafe_files)
       {
 	STRLIST tmp;
 
 	for(tmp=unsafe_files;tmp;tmp=tmp->next)
-	  check_permissions(tmp->d,0);
+	  check_permissions(tmp->d,0,0);
 
 	free_strlist(unsafe_files);
       }
 
+    if(extensions)
+      {
+	STRLIST tmp;
+
+	for(tmp=extensions;tmp;tmp=tmp->next)
+	  check_permissions(tmp->d,1,0);
+
+	free_strlist(extensions);
+      }
+
     if( may_coredump && !opt.quiet )
 	log_info(_("WARNING: program may create a core file!\n"));
 
@@ -1382,7 +1392,7 @@ main( int argc, char **argv )
     /* set the random seed file */
     if( use_random_seed ) {
 	char *p = make_filename(opt.homedir, "random_seed", NULL );
-	check_permissions(p,0);
+	check_permissions(p,0,0);
 	set_random_seed_file(p);
 	m_free(p);
     }
diff --git a/g10/keydb.c b/g10/keydb.c
index f4888bf3a..699960c3d 100644
--- a/g10/keydb.c
+++ b/g10/keydb.c
@@ -114,7 +114,7 @@ keydb_add_resource (const char *url, int force, int secret)
     else
 	filename = m_strdup (resname);
 
-    check_permissions(filename,0);
+    check_permissions(filename,0,0);
 
     if (!force)
 	force = secret? !any_secret : !any_public;
diff --git a/g10/keyserver.c b/g10/keyserver.c
index 3af9d2962..aa5ee5fc9 100644
--- a/g10/keyserver.c
+++ b/g10/keyserver.c
@@ -294,7 +294,7 @@ keyserver_spawn(int action,STRLIST list,u32 (*kidlist)[2],int count,int *prog)
     ret=exec_write(&spawn,command,NULL,0,0);
 
   if(ret)
-    goto fail;
+    return ret;
 
   fprintf(spawn->tochild,"# This is a gpg keyserver communications file\n");
   fprintf(spawn->tochild,"VERSION %d\n",KEYSERVER_PROTO_VERSION);
diff --git a/g10/main.h b/g10/main.h
index 4a94bdeb9..9d23bb992 100644
--- a/g10/main.h
+++ b/g10/main.h
@@ -67,7 +67,7 @@ int openpgp_cipher_test_algo( int algo );
 int openpgp_pk_test_algo( int algo, unsigned int usage_flags );
 int openpgp_pk_algo_usage ( int algo );
 int openpgp_md_test_algo( int algo );
-int check_permissions(const char *path,int checkonly);
+int check_permissions(const char *path,int extension,int checkonly);
 
 /*-- helptext.c --*/
 void display_online_help( const char *keyword );
diff --git a/g10/misc.c b/g10/misc.c
index 9240f12e7..acbd78d88 100644
--- a/g10/misc.c
+++ b/g10/misc.c
@@ -339,35 +339,53 @@ openpgp_md_test_algo( int algo )
 }
 
 int
-check_permissions(const char *path,int checkonly)
+check_permissions(const char *path,int extension,int checkonly)
 {
 #if defined(HAVE_STAT) && !defined(HAVE_DOSISH_SYSTEM)
-
+  char *tmppath;
   struct stat statbuf;
+  int ret=1;
   int isdir=0;
 
   if(opt.no_perm_warn)
     return 0;
 
+  if(extension && path[0]!=DIRSEP_C)
+    {
+      if(strchr(path,DIRSEP_C))
+	tmppath=make_filename(path,NULL);
+      else
+	tmppath=make_filename(GNUPG_LIBDIR,path,NULL);
+    }
+  else
+    tmppath=m_strdup(path);
+
   /* It's okay if the file doesn't exist */
-  if(stat(path,&statbuf)!=0)
-    return 0;
+  if(stat(tmppath,&statbuf)!=0)
+    {
+      ret=0;
+      goto end;
+    }
 
   isdir=S_ISDIR(statbuf.st_mode);
 
-  /* The user doesn't own the file */
-  if(statbuf.st_uid != getuid())
+  /* Per-user files must be owned by the user.  Extensions must be
+     owned by the user or root. */
+  if((!extension && statbuf.st_uid != getuid()) ||
+     (extension && statbuf.st_uid!=0 && statbuf.st_uid!=getuid()))
     {
       if(!checkonly)
 	log_info(_("Warning: unsafe ownership on %s \"%s\"\n"),
-		 isdir?"directory":"file",path);
-      return 1;
+		 isdir?"directory":extension?"extension":"file",path);
+      goto end;
     }
 
   /* This works for both directories and files - basically, we don't
      care what the owner permissions are, so long as the group and
-     other permissions are 0. */
-  if((statbuf.st_mode & (S_IRWXG|S_IRWXO)) != 0)
+     other permissions are 0 for per-user files, and non-writable for
+     extensions. */
+  if((extension && (statbuf.st_mode & (S_IWGRP|S_IWOTH)) !=0) ||
+     (!extension && (statbuf.st_mode & (S_IRWXG|S_IRWXO)) != 0))
     {
       char *dir;
 
@@ -377,22 +395,30 @@ check_permissions(const char *path,int checkonly)
          directory /, but for the sake of sanity, I'm stopping at one
          level down. */
 
-      dir=make_dirname(path);
+      dir=make_dirname(tmppath);
       if(stat(dir,&statbuf)==0 && statbuf.st_uid==getuid() &&
 	 S_ISDIR(statbuf.st_mode) && (statbuf.st_mode & (S_IRWXG|S_IRWXO))==0)
 	{
 	  m_free(dir);
-	  return 0;
+	  ret=0;
+	  goto end;
 	}
 
       m_free(dir);
 
       if(!checkonly)
 	log_info(_("Warning: unsafe permissions on %s \"%s\"\n"),
-		 isdir?"directory":"file",path);
-      return 1;
+		 isdir?"directory":extension?"extension":"file",path);
+      goto end;
     }
 
+  ret=0;
+
+ end:
+  m_free(tmppath);
+
+  return ret;
+
 #endif /* HAVE_STAT && !HAVE_DOSISH_SYSTEM */
 
   return 0;
diff --git a/g10/photoid.c b/g10/photoid.c
index 828514e8d..ab9025c73 100644
--- a/g10/photoid.c
+++ b/g10/photoid.c
@@ -236,10 +236,7 @@ void show_photo(const struct user_attribute *attr,PKT_public_key *pk)
   command[PHOTO_COMMAND_MAXLEN-1]='\0';
 
   if(exec_write(&spawn,NULL,command,1,1)!=0)
-    {
-      exec_finish(spawn);
-      goto fail;
-    }
+    goto fail;
 
   fwrite(attr->data,attr->len,1,spawn->tochild);
 
diff --git a/g10/tdbio.c b/g10/tdbio.c
index 8be44a790..1fa087e46 100644
--- a/g10/tdbio.c
+++ b/g10/tdbio.c
@@ -447,7 +447,7 @@ tdbio_set_dbname( const char *new_dbname, int create )
 		      : make_filename(opt.homedir,
 		                      "trustdb" EXTSEP_S "gpg", NULL );
 
-    check_permissions(fname,0);
+    check_permissions(fname,0,0);
 
     if( access( fname, R_OK ) ) {
 	if( errno != ENOENT ) {