Tags:
create new tag
, view all tags

Bug: Race condition during .htpasswd file updates

The .htpasswd updates have an inherent race condition which could result in corruption of the password file.

This has not occurred at our site! But it is a bad race condition.

User additions, password updates, and password resets all look like this:

    my $text = &TWiki::Store::readFile( $TWiki::htpasswdFilename );
    ... #do something which changes $text
    &TWiki::Store::saveFile( $TWiki::htpasswdFilename, $text );

This is very bad- neither readFile nor saveFile acquire any kind of file lock, and (even if they did), the lock would not be maintained from time of initial read to final update write.

It should be:

  • open file with read&update flag
  • lock file
  • read in contents of file
  • perform updates
  • overwrite file with new contents
  • release lock
  • close file

Possible failure scenarios are:

  • last update wins (last to save overwrites other concurrent changes)
  • EOF-read during buffered write (file truncation/corruption)
  • (reattemptable) login validation error

Likelihood of problems is directly proportional to number of writing agents (httpd instances) and the size of htpasswd file. Also, open/write may be acquiring implicit locks on some platforms (windows, depending on perl version & such).

Test case

lots of simultaneous updates to password/user info using large htpasswd file.

Environment

TWiki version: 20011201
TWiki plugins: none
Server OS: solaris 2.7
Web server: apache 1.3.12
Perl version: 5.61
Client OS: varied
Web Browser: varied

-- EdwardJajko - 07 May 2002

Follow up

This is a bug. Wondering if it manifests itself. At TWiki.org we have currently 6938 users (over 8000 if you include the bogus registrations) and we never experienced this race condition. Does it have a probability similar to getting hit by a meteor?

A correct fix would be to lock the update before the read (if not locked), and release the lock when done. If locked, wait for a random number of miliseconds and try again.

-- PeterThoeny - 02 Sep 2003

Probably doesn't happen much with creating users, after all there have only been 8000 or so new user creations in several years at TWiki.org - however could still happen on an intranet perhaps. At least the lock is not held across a user interaction, which is where DBs tend to have real locking problems.

Unix and Windows locking is somewhat problematic - it might be a good idea for portability to instead use 'optimistic concurrency control' in which you just detect the fact that a collision has occurred and make the losing user retry. This can be done by taking timestamp or checksum of the file when first reading it, and then re-checking the timestamp/checksum right before you try to save it. This works best with databases, where you can put in an SQL WHERE clause that prevents the row being updated if the timestamp has changed - not sure how to make this work 100% correctly with .htpasswd files, since I think this check would just reduce the duration of the vulnerability rather than avoid it. However, it could be done as a 99.9% solution quite easily by redoing the readFile, just before the storeFile, and doing a simple string comparison of the data read - this would be slower for very large password files but less prone to problems than locking.

Alternatively we could just do locking via Perl for Unix/Linux and let other platforms support this as and when possible - Windows often uses built-in locking for every file access but I'm not sure if it applies here.

This should probably be addressed if we make it easier for users to change the password without involving an administrator. I know some work has been done on this.

-- RichardDonkin - 02 Sep 2003

I don't think concencus has been reached on the way forward for this, so I am unassigning it from DakarRelease. - CrawfordCurrie

Fix record

ChangeProposalForm
TopicClassification BugReport
TopicSummary The .htpasswd updates have an inherent race condition which could result in corruption of the password file.
CurrentState UnderInvestigation
OutstandingIssues Consensus not reached on whether this is worth fixing or not.
RelatedTopics

InterestedParties

ProposedFor

TWikiContributors

Edit | Attach | Watch | Print version | History: r8 < r7 < r6 < r5 < r4 | Backlinks | Raw View | Raw edit | More topic actions
Topic revision: r8 - 2005-02-15 - SamHasler
 
  • Learn about TWiki  
  • Download TWiki
This site is powered by the TWiki collaboration platform Powered by Perl Hosted by OICcam.com Ideas, requests, problems regarding TWiki? Send feedback. Ask community in the support forum.
Copyright © 1999-2017 by the contributing authors. All material on this collaboration platform is the property of the contributing authors.