Ticket #871 (fixed bug)
Caching breaks on concurrent access, possible information disclosure
- Created: 2013-07-09 08:36:39
- Reported by: Bluewind
- Assigned to: Franz
- Milestone: 1.5.4
- Component: caching
- Priority: normal
Cache files are opened with fopen in "wb" mode and no exclusive flag or locking. Therefore multiple concurrent processes will be able to open the same cache file at the same time.
If both processes have different data to write to the file and the longer data gets written before the shorter one, the excessive part of the longer data will not be removed and it will be displayed in the output.
Happened on the Arch Linux BBS bbs.archlinux.org/viewtopic.php?id=166425 for the cache_bans.php file.
You should either use some variable cache like memcache that can handle concurrent access or use a lock file or use O_EXCL when opening the cache file to writing.
adaur 2013-07-09 11:07:42
- Milestone set to 1.5.4.
Thanks for the report. Should we use "x" or "c" instead of "wb" ?
Bluewind 2013-07-09 12:25:30
I'd go with using fopen("wb"), flock(LOCK_EX), ftruncate(0), fwrite(), flock(LOCK_UN), fclose().
You should also create a dedicated function instead of duplicating the fopen, fwrite, fclose code everywhere.
Edit: You could use fopen("x"), but that would introduce another race condition when you check if the file exists before including it.
Also know that flock might not work over NFS.
adaur 2013-07-10 09:09:03
Thanks for the details, we'll look into it. Have a nice day!
Franz 2013-07-11 20:12:03
- Owner set to adaur.
Can you take care of that?
Thanks for the report, good catch!
Franz 2013-08-07 16:23:18
- Owner changed from adaur to Franz.
Okay, I just implemented locking when writing the cache files.
Can somebody please take a look at and test my implementation? It seemed to work here, but you never know. Plus it's obviously hard to test the exact scenario that was mentioned by the ticket creator.
Bluewind 2013-08-07 16:56:17
Looks good, you can easily test this in a php shell (php -a).
Franz 2013-08-07 21:25:52
Yeah, I tested the writing of the files. What I cannot test is having the two concurrent accesses of the file.
Franz 2013-08-08 22:02:59
adaur 2013-08-09 19:49:57
Sorry about this one, Franz. I've been on holidays for too long .
Franz 2013-08-09 20:46:25
No problem. Does the code look okay to you?
adaur 2013-08-09 21:00:39
I don't know much how to handle files in PHP but it looks OK for all I know.
Update extern.php???? (// Output feed as PHP code; lines 439-451)
Franz 2013-08-10 16:16:54
Thank you, done.
Franz 2013-08-10 19:34:33
- Status changed from open to fixed.
Okay, this is running on our server now. Seems to work.
Visman 2013-08-11 08:00:29
$pun_user - an excess global variable in generate_quickjump_cache()
Franz 2013-08-11 21:24:19