From: Frank Lichtenheld <frank@lichtenheld.de>
Date: Mon, 17 Mar 2008 16:54:05 GMT
Subject: Re: [RFC/PATCH 2/3] gitweb: Support caching projects list
Message-ID: <20080317165405.GD18624@mail-vs.djpig.de>
In-Reply-To: <1205766570-13550-3-git-send-email-jnareb@gmail.com>

On Mon, Mar 17, 2008 at 04:09:29PM +0100, Jakub Narebski wrote:
> From: Petr Baudis <pasky@suse.cz>
> $projlist_cache_lifetime gitweb configuration variable is introduced,
> by default set to zero. If set to non-zero, it describes the number of
> minutes for which the cache remains valid. Only single project root
> per system can use the cache. Any script running with the same uid as
> gitweb can change the cache trivially - this is for secure
> installations only.

The more subtle threat is the fact that anyone with writing
rights to /tmp can give gitweb any data he wants if the file doesn't
exist yet.

At the very least you should:

 - Allow to override /tmp (via ENV{TMPDIR} or via a configuration
   variable)
 - Advise people to change that to something that is not world-writable
 - Check if the file is owned by the uid gitweb is running under and
   not word-writable.

[...]
> +	my @projects;
> +	my $stale = 0;
> +	my $now = time();
> +	if ($cache_lifetime && -f $cache_file &&
> +	    stat($cache_file)->mtime + $cache_lifetime * 60 > $now &&
> +	    open(my $fd, '<', $cache_file)) {
> +		$stale = $now - stat($cache_file)->mtime;

One stat() call instead of three would be better for performance.

Gruesse,
-- 
Frank Lichtenheld <frank@lichtenheld.de>
www: http://www.djpig.de/