bogoutil and realpath

[David Relson]

On Fri, 22 Apr 2005 00:45:04 +0200
Matthias Andree wrote:

> Matthias Andree <matthias.andree at gmx.de> writes:
> 
> >> By the way, have you read the manpage of realpath?  It says "do not
> >> use" :0
> >
> > The GNU libc manual pages in their usual vituperating politcal agenda. I
> > don't care, realpath() isn't going away anywhen soon, it's in POSIX 2004
> > and is not marked "legacy".
> 
> Well, it seems GNU has a reason to bitch - their HURD has no PATH_MAX
> since that value is dynamic.
> 
> So we'll stat and compare st_dev and st_ino - these are unique on a
> machine and specific for a directory entry.
> 
> How about this patch?
>   (I'm not committing this now to avoid conflicting changes.)
> 
> BTW, this has an implicit TOCTOU race as it checks earlier than it's
> used but if someone feels like hacking symlinks for BOGOHOME underneath
> a running bogofilter, their business if they suffer holes in their feet.
> 
> Really bedtime now.
> 
> Index: src/datastore_db.c
> ===================================================================
> RCS file: /cvsroot/bogofilter/bogofilter/src/datastore_db.c,v
> retrieving revision 1.202
> diff -u -p -r1.202 datastore_db.c
> --- a/src/datastore_db.c	11 Apr 2005 19:06:33 -0000	1.202
> +++ b/src/datastore_db.c	21 Apr 2005 22:42:17 -0000
> @@ -276,18 +276,6 @@ static void handle_free(/*@only@*/ dbh_t
>      return;
>  }
>  
> -static bool checkpath(const char *path, char *norm)
> -{
> -    if (realpath(path, norm) != NULL)
> -	return true;
> -    else {
> -	print_error(__FILE__, __LINE__,
> -		    "error: cannot normalize path \"%s\": %s",
> -		    path, strerror(errno));
> -	return false;
> -    }
> -}
> -
>  /** Initialize data base, configure some lock table sizes
>   * (which can be overridden in the DB_CONFIG file)
>   * and lock the file to tell other parts we're initialized and
> @@ -295,16 +283,23 @@ static bool checkpath(const char *path, 
>   */
>  void *dbe_init(bfpath *bfp)
>  {
> -    char norm_dir[PATH_MAX+1]; /* check normalized directory names */
> -    char norm_home[PATH_MAX+1];/* see man realpath(3) for details */
> -
> +    struct stat std, sth;
>      dbe_t *env;
>  
> -    if ( !checkpath(bfp->dirname, norm_dir) ||
> -	 !checkpath(bogohome, norm_home))
> +    if (stat(bfp->dirname, &std)) {
> +	fprintf(stderr, "ERROR: cannot stat \"%s\": %s, aborting.\n",
> +		bfp->dirname, strerror(errno));
> +	exit(EX_ERROR);
> +    }
> +
> +    if (stat(bogohome, &sth)) {
> +	fprintf(stderr, "ERROR: cannot stat \"%s\": %s, aborting.\n",
> +		bfp->dirname, strerror(errno));
>  	exit(EX_ERROR);
> +    }
>  
> -    if (strcmp(norm_dir, norm_home) != 0)
> +    if (std.st_dev != sth.st_dev
> +	     || sth.st_ino != sth.st_ino)
>      {
>  	fprintf(stderr,
>  		"ERROR: only one database _environment_ (directory) can be used at a time.\n"
> @@ -312,8 +307,7 @@ void *dbe_init(bfpath *bfp)
>  	fprintf(stderr,
>  		"If you need multiple wordlists in different directories,\n"
>  		"you cannot use the transactional interface, but you must configure\n"
> -		"the non-transactional interface, i. e. ./configure --disable-transactions\n"
> -		"then type make clean, after that rebuild and install as usual.\n"
> +		"the non-transactional interface.\n"
>  		"Note that the data base will no longer be crash-proof in that case.\n"
>  		"Please accept our apologies for the inconvenience.\n");
>  	fprintf(stderr,
> @@ -322,7 +316,7 @@ void *dbe_init(bfpath *bfp)
>      }
>  
>      dsm_init(bfp);
> -    
> +
>      env = dsm->dsm_env_init(bfp);
>  
>      return env;

Let's keep dbe_init() short and simple and put all the path checking
stuff into check_path().  I'm done with the changes I have.  Feel free
to test to add yours in now (or in the morning).  If things go well, I
can do a release Saturday.

Cheers!