On Thu, 2005-08-25 at 01:35 +0200, Sylvain Beucler wrote:
> Hi,
>
> When you use pserver, you have to specify all the CVS repositories you
> give people access to on the command line, each in a --allow-root
> option. On systems like Savannah with around 5000 public repositories,
> this gets over the command line limit of around 130KB.
>
> I'm discussing that at [EMAIL PROTECTED] An old solution was to add
> --allow-root-regexp
> (https://ccvs.cvshome.org/issues/show_bug.cgi?id=41). There is also a
> patch in the old Savannah project bug tracker to implement
> --allow-root-file
> (http://lists.gnu.org/archive/html/savannah-dev/2002-04/msg00373.html). So
> apparently work was done in 2001 but never integrate.
>
>
> So my question is: I was once sent some Gna! documentation and I saw a
> reference to a Perl suid script for pserver access. I have no clue on
> how you implemented pserver access, and I wondered if you have some
> comments on the preferred way to fix the issue, because I may well
> work finishing the --allow-root-regexp integration :)
It just happened that I mentioned it 10 minutes ago in another
thread :).
We modified --allow-root to accept a very simple substring matching.
In other words, "--allow-root /var/cvs/" will accept any repository path
which begins with "/var/cvs/" (and takes care of .. tricks and such).
This is backwards compatible with the original --allow-root.
Patch for Woody's CVS attached (not ported to Sarge yet), about 10
lines of code without comments.
--- cvs-1.11.1p1/src/root.c.orig Fri Jan 16 16:24:58 2004
+++ cvs-1.11.1p1/src/root.c Fri Jan 16 16:59:55 2004
@@ -242,6 +242,8 @@
char *arg;
{
int i;
+ int arg_len;
+ char *c;
if (root_allow_count == 0)
{
@@ -259,9 +261,46 @@
error_exit ();
}
+ /* 2004-01-16 [EMAIL PROTECTED]
+ To prevent funny tricks with UTF-8 that could invalidate the
+ very basic next test against possible malicious paths, we
+ are quite restrictive on the input charset. */
+ for (c = arg; *c != '\0'; c++)
+ if (*c < 0)
+ {
+ printf ("\
+error 0 Non-ASCII characters not allowed in cvsroot path\n");
+ error_exit ();
+ }
+
+ /* 2004-01-16 [EMAIL PROTECTED]
+ Since next test only checks a path substring, we need to be
+ careful with '..' pseudo paths. Actually we just reject any
+ substring matching '..'; we'll got some false positives, but
+ on obviously strange looking paths (too many dots!). */
+ if (strstr(arg, "..") != NULL)
+ {
+ printf ("\
+error 0 Unsafe path elements in cvsroot\n");
+ error_exit ();
+ }
+
+ /* 2004-01-16 [EMAIL PROTECTED]
+ Modified the original test to check if the cvsroot 'begins
+ with' one of the allowed cvsroot. It makes up a very simple
+ pattern matching system, together with the previous security
+ tests. */
+ arg_len = strlen(arg);
for (i = 0; i < root_allow_count; ++i)
- if (strcmp (root_allow_vector[i], arg) == 0)
+ {
+ char *allowed;
+ int allowed_len;
+
+ allowed = root_allow_vector[i];
+ allowed_len = strlen(allowed);
+ if (arg_len >= allowed_len && strncmp (arg, allowed, allowed_len) == 0)
return 1;
+ }
return 0;
}
