From owner-freebsd-cvsweb@FreeBSD.ORG Sat May 25 13:46:51 2013 Return-Path: Delivered-To: freebsd-cvsweb@FreeBSD.org Received: from mx1.freebsd.org (mx1.FreeBSD.org [8.8.178.115]) by hub.freebsd.org (Postfix) with ESMTP id 6300A1CF for ; Sat, 25 May 2013 13:46:51 +0000 (UTC) (envelope-from jem@iki.fi) Received: from sampo.toomanyhops.net (unknown [IPv6:2001:14b8:1ff:1:222:2ff:fe00:18c3]) by mx1.freebsd.org (Postfix) with ESMTP id CCFE51E7 for ; Sat, 25 May 2013 13:46:50 +0000 (UTC) Received: from [IPv6:2001:14b8:1ff:1:21f:d0ff:fe51:b7f6] (pandora.toomanyhops.net [IPv6:2001:14b8:1ff:1:21f:d0ff:fe51:b7f6]) by sampo.toomanyhops.net (Postfix) with ESMTPS id B26972A2CD for ; Sat, 25 May 2013 16:46:47 +0300 (EEST) Message-ID: <51A0C0C7.4080104@iki.fi> Date: Sat, 25 May 2013 16:46:47 +0300 From: Johan Myreen User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130519 Thunderbird/17.0.6 MIME-Version: 1.0 To: freebsd-cvsweb@FreeBSD.org Subject: Bug related to ACLs in cvsweb Content-Type: multipart/mixed; boundary="------------000904040501080507090103" X-BeenThere: freebsd-cvsweb@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: CVS Web maintenance mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 25 May 2013 13:46:51 -0000 This is a multi-part message in MIME format. --------------000904040501080507090103 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Hi. Due to a Perl misfeature, cvsweb.cgi does not work correctly if Access Control Lists (ACLs) are in use. The script includes the pragma use filetest qw(access); This pragma changes how Perl does permission checks on files and directories; instead of using stat(), permissions are checked using access(). The problem is that the script uses the cached value of a stat() call to check permissions, using the special filehandle _. When the filetest 'access' pragma is in use, the -r $file, -w $file and -x $file tests do not set the cache (because no call to stat() is made). What's worse, when the stat cache is set, e.g. as a result of -d $file, it contains the wrong value for a -r _ test. The stat cache contains the traditional rwx mode bits, and does not reflect any additional permissions granted by the ACL. See: http://perldoc.perl.org/filetest.html ACLs are very useful when used with cvsweb. You can grant the 'www-data' user read permission to the repository files without opening them up to all users on the server (with chmod o+r). Of course, you could add user 'www-data' to the 'cvs' group, but that would mean 'www-data' would have write permission to the repository. Patch attached. Keywords: cvsweb acl bug filetest access Johan Myréen jem@iki.fi --------------000904040501080507090103 Content-Type: text/x-patch; name="cvsweb.diff" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="cvsweb.diff" diff -u cvsweb-3.0.6-orig/cvsweb.cgi cvsweb-3.0.6/cvsweb.cgi --- cvsweb-3.0.6-orig/cvsweb.cgi 2005-09-25 23:28:51.000000000 +0300 +++ cvsweb-3.0.6/cvsweb.cgi 2013-05-25 15:01:58.000000000 +0300 @@ -990,10 +990,11 @@ next if ($input{hidecvsroot} && $where eq '/' && $file eq 'CVSROOT'); # Is it a directory? - my $isdir = -d catdir($fullname, $file); + my $subdir = catdir($fullname, $file); + my $isdir = -d $subdir; # Ignore non-readable files and directories? - next if ($input{hidenonreadable} && (! -r _ || ($isdir && ! -x _))); + next if ($input{hidenonreadable} && (! -r $subdir || ($isdir && ! -x $subdir))); my $attic = ''; if ($file =~ s|^Attic/||) { @@ -1459,8 +1460,8 @@ ($filename) = (catfile($dirname, $filename) =~ VALID_PATH) or next; # untaint my ($file) = catfile($fullname, $filename); - next if ($filename !~ /,v$/o || !-f $file || !-r _); - my $modtime = -M _; + next if ($filename !~ /,v$/o || !-f $file || !-r $file); + my $modtime = -M $file; if (!defined($lastmod) || $modtime < $lastmodtime) { ($lastmod = $filename) =~ s/,v$//; $lastmodtime = $modtime; @@ -1718,7 +1719,7 @@ my ($command) = @_; for my $d (@command_path) { my $cmd = catfile($d, $command); - return $cmd if (-x $cmd && !-d _); + return $cmd if (-x $cmd && !-d $cmd); } return ''; } @@ -1753,7 +1754,7 @@ my $mimetype = $MTYPES{$suffix}; $mimetype ||= $MimeTypes->mimeTypeOf($fullname) if defined($MimeTypes); - if (!$mimetype && $suffix ne '*' && -f $mime_types && -r _) { + if (!$mimetype && $suffix ne '*' && -f $mime_types && -r $mime_types) { my $fh = do { local (*FH); }; if (open($fh, $mime_types)) { my $re = sprintf('^\s*(\S+\/\S+)\s.+\b%s\b', quotemeta($suffix)); @@ -2454,7 +2455,7 @@ # Note: last modified files from subdirs returned by # findLastModifiedSubdirs() come without the ,v suffix so they're not # found here, but have already been checked for readability. *cough* - if (-r $files[$i] || !-e _) { + if (-r $files[$i] || !-e $files[$i]) { $i++; } else { push(@unreadable, splice(@files, $i, 1)); --------------000904040501080507090103--