Jeremy Chadwick dijo [Fri, Sep 26, 2008 at 07:37:19PM -0700]:
> > 0 [EMAIL PROTECTED]/home/gwolf# cherokee
> > Couldn't open '/var/log/cherokee/cherokee.access' for appending
> > 2 [EMAIL PROTECTED]/home/gwolf# ls -l /var/log/cherokee/cherokee.*
> > -rw-r--r-- 1 root root 1143 2008-09-26 14:11
> > /var/log/cherokee/cherokee.access
> > -rw-r--r-- 1 root root 388 2008-09-26 14:11
> > /var/log/cherokee/cherokee.error
>
> IMHO, good. Reversing the order of items 2 and 3 was the right thing to
> do. There is a major security issue with opening log files as root
> before dropping credentials. You cover the reason below:
> (...)
> Incorrect. Take the following scenario (I do not speak Cherokee, so
> this is Apache syntax), which users often want (because they want to be
> able to watch their web traffic in real-time):
> (...)
> <VirtualHost ...>
> CustomLog /home/jdc/www_logs/access.log combined
> </VirtualHost>
>
> drwx--x--x 14 jdc users 512 26 Sep 03:03 /home/jdc/
> drwx------ 5 jdc users 512 26 Sep 03:03 /home/jdc/www_logs/
> -rw-r--r-- 1 root wheel 5033 26 Sep 09:18
> /home/jdc/www_logs/access.log
>
> All user "jdc" has to do is:
>
> $ cd ~/www_logs
> $ rm -f access.log
> $ ln -s /etc/passwd access.log
>
> And then wait for the webserver to re-open the logfiles (such as upon
> restart). Let the games begin.
Of course - That's why a server won't ever (or shouldn't ever) write
to a link. So, I think something like this could be a better way out -
Please note that I'm only copying code around in the code, I didn't
even try compiling this:
diff --git a/cherokee/logger_writer.c b/cherokee/logger_writer.c
index 3c68ca4..d12fe72 100644
--- a/cherokee/logger_writer.c
+++ b/cherokee/logger_writer.c
@@ -216,6 +216,8 @@ launch_logger_process (cherokee_logger_writer_t *writer)
ret_t
cherokee_logger_writer_open (cherokee_logger_writer_t *writer)
{
+ int r;
+ struct stat info;
ret_t ret;
switch (writer->type) {
@@ -232,6 +234,14 @@ cherokee_logger_writer_open (cherokee_logger_writer_t
*writer)
return ret_ok;
case cherokee_logger_writer_file:
+ /* Check against symlink attacks */
+ r = stat (writer->filename.buf, &info);
+ /* Disregard if the stat failed - The file does not exist, so
everything is fine */
+ if (r == 0 and S_ISLINK(info.st_mode) == 0) {
+ PRINT_MSG ("%s unsafe for opening: Is a symlink\n",
writer->filename.buf);
+ return ret_error
+ }
+
writer->fd = open (writer->filename.buf, O_APPEND | O_WRONLY |
O_CREAT | O_LARGEFILE, 0
if (writer->fd == -1) {
PRINT_MSG ("Couldn't open '%s' for appending\n",
writer->filename.buf);
So in this case, you would not modify even a same-owner or
world-writable file symlinked from the log dir.
In any case - I think a patch as this one could/should be applied,
even if the opening order is not set to its previous status.
Greetings,
--
Gunnar Wolf - [EMAIL PROTECTED] - (+52-55)5623-0154 / 1451-2244
PGP key 1024D/8BB527AF 2001-10-23
Fingerprint: 0C79 D2D1 2C4E 9CE4 5973 F800 D80E F35A 8BB5 27AF
_______________________________________________
Cherokee mailing list
[email protected]
http://lists.octality.com/listinfo/cherokee
