[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Patch to fix val-tags bug

From: krellan
Subject: Patch to fix val-tags bug
Date: Tue, 3 Jul 2001 12:01:13 -0700

Here is a patch I have made to CVS.

It affects two files:

It fixes the dreaded "can't write val-tags" bug, by turning it into a
harmless warning!

This bug affects guest users (with read-only access) who attempt to
check out any revision of code that is not the latest revision.  I hope to
fix this with my small patch.

(Do a Google search on "val-tags", to see how many people have been
bitten by this bug....)

I believe this is what the original author intended (in src/tag.c), but
because of this bug, the warning was never able to be printed.  CVS exited
with a fatal error and did not print this warning or continue the checkout.

Here's what I believe is happening:

The user tries to checkout code with a tag for the particular revision
of interest.
CVS checks to see if the tag is cached in val-tags, and if it isn't,
it scans the tree and searches for the tag.
Once the tag is found, it attempts to write the tag into val-tags.
This fails, because guests don't have write access, and CVS aborts.
The common workaround is to give world write access to the val-tags file,
which is a potential security hole (DoS by expanding this file to fill
up the entire disk, for instance).

In tag.c (tag_check_valid), CVS uses dbm_open to open the val-tags file
with read/write access.
In myndbm.c (mydbm_open), this requested access is ignored, and the file
is instead opened read-only!
This causes tag.c to falsely believe that write permission has been
granted to this file.
It will then assume that it is OK to write to this file.

When tag.c attempts to store the newly found tag into the val-tags file,
it calls dbm_store to store the new data item.
This is simply cached in memory and is not actually written until
dbm_close is called.
In dbm_close, if any changes have been made, it will reopen the file
for write access and write the changes... and if there is any error
writing the file (as there will be for read-only users), it
will *abort* the entire CVS program with the "can't write" error message!

Even though the search for the tag was successful, and CVS is ready to
check out the files to the user, it doesn't get a chance to happen
because the program gets aborted at this point.

My fix is to change mydbm_open to properly check permissions on files
that it opens, and return an error at that point.
This lets the code in tag.c properly treat the val-tags file as an
optional speed-enhancing cache, and not malfunction if there are
errors writing to that file.

I searched for other uses of dbm_open within CVS, and the only other
instance where a file is opened for write access is in
mkmodules.c (write_dbmfile).  Guest users with read-only access won't be
running mkmodules, though, so this extra permissions check should not be
a problem.  I've tested this patch against a live source tree, and it
worked well for me.

Is this patch acceptable for inclusion into CVS?

Josh Lehan

diff -urN ccvs-old/lib/system.h ccvs-new/lib/system.h
--- ccvs-old/lib/system.h       Wed Feb 14 18:53:07 2001
+++ ccvs-new/lib/system.h       Tue Jul  3 11:25:56 2001
@@ -508,6 +508,7 @@
 #define FOPEN_BINARY_READ ("rb")
 #define FOPEN_BINARY_WRITE ("wb")
 #ifdef O_BINARY
diff -urN ccvs-old/src/myndbm.c ccvs-new/src/myndbm.c
--- ccvs-old/src/myndbm.c       Tue Jul  4 12:45:20 2000
+++ ccvs-new/src/myndbm.c       Tue Jul  3 11:26:02 2001
@@ -33,7 +33,8 @@
     FILE *fp;
     DBM *db;
-    fp = CVS_FOPEN (file, FOPEN_BINARY_READ);
+       /* test for file write access if attempting to open file non-readonly */
+    fp = CVS_FOPEN (file, ((flags & O_ACCMODE) != O_RDONLY) ? 
     if (fp == NULL && !(existence_error (errno) && (flags & O_CREAT)))
        return ((DBM *) 0);

reply via email to

[Prev in Thread] Current Thread [Next in Thread]