Forum

Bug and security fixes

Stephen R. van den Berg
21 January 2011, 17:29
The following patches can be fetched from git using the following sequence:

git clone git://git.cuci.nl/hiawatha
git fetch git://git.cuci.nl/hiawatha BuGless:BuGless
git checkout BuGless
git log

Patch in full relative to 7.4:
----------------------------------------------------------------------------------
From 680af9de34ee3b3463c8352492579c115fa38c99 Mon Sep 17 00:00:00 2001
From: Stephen R. van den Berg <srb@cuci.nl>
Date: Wed, 10 Nov 2010 14:24:39 +0100
Subject: Add gitignore

---
.gitignore | 17 +++++++++++++++++
1 files changed, 17 insertions(+), 0 deletions(-)
create mode 100644 .gitignore

diff --git a/.gitignore b/.gitignore
new file mode 100644
index 0000000..b951840
--- /dev/null
+++ b/.gitignore
@@ -0,0 +1,17 @@
+*.o
+build
+Makefile
+config.h
+config.status
+*.log
+cgi-wrapper
+hiawatha
+wigwam
+debian/files
+debian/*.debhelper
+debian/*.substvars
+debian/hiawatha
+.deps
+php-fcgi
+ssi-cgi
+stamp-*
--
1.7.2.3


From 4cafea842ce3603fc614154d0e7ee5d31dc39909 Mon Sep 17 00:00:00 2001
From: Stephen R. van den Berg <srb@cuci.nl>
Date: Fri, 21 Jan 2011 02:50:44 +0100
Subject: Fix handling of error-handlers

---
target.c | 16 +++++++---------
1 files changed, 7 insertions(+), 9 deletions(-)

diff --git a/target.c b/target.c
index 2adc536..6cec305 100644
--- a/target.c
+++ b/target.c
@@ -774,6 +774,7 @@ int execute_cgi(t_session *session) {
break;
}
} else if (session->host->trigger_on_cgi_status && ((code = strcasestr(cgi_info.input_buffer, "Status: ")) != NULL)) {
+ int status = -1;
line = code += 8;

retval = -1;
@@ -781,7 +782,7 @@ int execute_cgi(t_session *session) {
if ((*line == '\r') || (*line == ' ')) {
c = *line;
*line = '\0';
- retval = str2int(code);
+ status = str2int(code);
*line = c;

*(new_line + 2) = '\r';
@@ -790,18 +791,15 @@ int execute_cgi(t_session *session) {
line++;
}

- if (retval <= 0) {
+ if (status <= 0) {
log_error(session, "invalid status code received from CGI");
retval = 500;
break;
}
- session->return_code = retval;
- if (retval == 200) {
- if (send_header(session) == -1) {
- retval = rs_DISCONNECT;
- break;
- }
- } else {
+ retval = 200;
+ session->return_code = status;
+ if (send_header(session) == -1) {
+ retval = rs_DISCONNECT;
break;
}
} else if (send_header(session) == -1) {
--
1.7.2.3


From cd30f3f23450eaa321ee1fbef8dd63ea0815072c Mon Sep 17 00:00:00 2001
From: Stephen R. van den Berg <srb@cuci.nl>
Date: Fri, 21 Jan 2011 16:53:38 +0100
Subject: Lock down security issues regarding chmod/chown/setuid.

---
hiawatha.c | 93 ++++---------------------------------------------------
libfs.c | 19 +++++------
log.c | 8 -----
monitor.c | 3 +-
serverconfig.c | 2 +-
target.c | 6 ---
6 files changed, 17 insertions(+), 114 deletions(-)

diff --git a/hiawatha.c b/hiawatha.c
index 80783b3..a5432bd 100644
--- a/hiawatha.c
+++ b/hiawatha.c
@@ -133,20 +133,21 @@ char *version_string = "Hiawatha v"VERSION
#endif
;

+#define LOGPERM (S_IRUSR|S_IWUSR|S_IRGRP|S_IWGRP)
/* Create all logfiles with the right ownership and accessrights
*/
void touch_logfiles(t_config *config, char *dir) {
t_host *host;

- touch_logfile(dir, config->system_logfile, 0640, config->server_uid, config->server_gid);
+ touch_logfile(dir, config->system_logfile, LOGPERM, config->server_uid, config->server_gid);
if (config->garbage_logfile != NULL) {
- touch_logfile(dir, config->garbage_logfile, 0640, config->server_uid, config->server_gid);
+ touch_logfile(dir, config->garbage_logfile, LOGPERM, config->server_uid, config->server_gid);
}
if (config->exploit_logfile != NULL) {
- touch_logfile(dir, config->exploit_logfile, 0640, config->server_uid, config->server_gid);
+ touch_logfile(dir, config->exploit_logfile, LOGPERM, config->server_uid, config->server_gid);
}
#ifdef DEBUG
- touch_logfile(dir, LOG_DIR"/debug.log", 0640, config->server_uid, config->server_gid);
+ touch_logfile(dir, LOG_DIR"/debug.log", LOGPERM, config->server_uid, config->server_gid);
#endif

host = config->first_host;
@@ -154,8 +155,8 @@ void touch_logfiles(t_config *config, char *dir) {
if (host->access_fileptr != NULL) {
fflush(host->access_fileptr);
}
- touch_logfile(dir, host->access_logfile, 0640, config->server_uid, config->server_gid);
- touch_logfile(dir, host->error_logfile, 0640, config->server_uid, config->server_gid);
+ touch_logfile(dir, host->access_logfile, LOGPERM, config->server_uid, config->server_gid);
+ touch_logfile(dir, host->error_logfile, LOGPERM, config->server_uid, config->server_gid);
host = host->next;
}
}
@@ -1926,86 +1927,6 @@ int run_server(t_settings *settings) {
}
#endif

- /* Create work directory
- */
- if (mkdir(config->work_directory, 0700) == -1) {
- if (errno != EEXIST) {
- fprintf(stderr, "Error creating work directory '%s'\n", config->work_directory);
- return -1;
-#ifndef CYGWIN
- } else if (chmod(config->work_directory, 0700) == -1) {
- fprintf(stderr, "Can't change access permissions of work directory '%s'\n", config->work_directory);
- return -1;
-#endif
- }
- }
-#ifndef CYGWIN
- if ((getuid() == 0) || (geteuid() == 0)) {
- if (chown(config->work_directory, config->server_uid, config->server_gid) == -1) {
- perror("chown(WorkDirectory)");
- return -1;
- }
- }
-#endif
-
- /* Create the upload directory for PUT requests
- */
- if (mkdir(config->upload_directory, 0733) == -1) {
- if (errno != EEXIST) {
- fprintf(stderr, "Error while creating UploadDirectory '%s'\n", config->upload_directory);
- return -1;
- }
- }
-
-#ifndef CYGWIN
- if (stat(config->upload_directory, &status) == -1) {
- perror("stat(UploadDirectory)");
- return -1;
- }
- access_rights = 01733;
- if (status.st_uid != 0) {
- if ((getuid() == 0) || (geteuid() == 0)) {
- if (chown(config->upload_directory, 0, 0) == -1) {
- perror("chown(UploadDirectory, 0, 0)");
- return -1;
- }
- } else {
- access_rights = 01333;
- }
- }
-
- if ((status.st_mode & 07777) != access_rights) {
- if (chmod(config->upload_directory, access_rights) == -1) {
- fprintf(stderr, "Can't change access permissions of UploadDirectory '%s'.\n", config->upload_directory);
- return -1;
- }
- }
-#endif
-
-#ifdef HAVE_MONITOR
- /* Create monitor cache directory
- */
- if (mkdir(config->monitor_directory, 0700) == -1) {
- if (errno != EEXIST) {
- fprintf(stderr, "Error creating monitor directory '%s'\n", config->monitor_directory);
- return -1;
-#ifndef CYGWIN
- } else if (chmod(config->monitor_directory, 0700) == -1) {
- fprintf(stderr, "Can't change access permissions of monitor directory '%s'\n", config->monitor_directory);
- return -1;
-#endif
- }
- }
-#ifndef CYGWIN
- if ((getuid() == 0) || (geteuid() == 0)) {
- if (chown(config->monitor_directory, config->server_uid, config->server_gid) == -1) {
- perror("chown(MonitorDirectory)");
- return -1;
- }
- }
-#endif
-#endif
-
/* Create logfiles
*/
#ifdef HAVE_CHROOT
diff --git a/libfs.c b/libfs.c
index 760e172..f1967a7 100644
--- a/libfs.c
+++ b/libfs.c
@@ -247,20 +247,17 @@ void touch_logfile(char *dir, char *file, mode_t mode, uid_t uid, gid_t gid) {
sprintf(logfile, "%s%s", dir, file);
}
if (logfile != NULL) {
- if ((fd = open(logfile, O_RDONLY)) == -1) {
- if ((fd = open(logfile, O_CREAT|O_APPEND, mode)) != -1) {
- if (getuid() == 0) {
- if (fchown(fd, uid, gid) == -1) {
- fprintf(stderr, "Warning: couldn't chown logfile %s\n", logfile);
- }
- }
- close(fd);
- } else {
- fprintf(stderr, "Warning: couldn't create logfile %s\n", logfile);
- }
+ uid_t oldeuid;
+ gid_t oldegid;
+ oldeuid = geteuid();
+ oldegid = getegid();
+ setegid(gid);seteuid(uid);
+ if ((fd = open(logfile, O_CREAT, mode)) != -1) {
+ fprintf(stderr, "Warning: couldn't create logfile %s\n", logfile);
} else {
close(fd);
}
+ seteuid(oldeuid);setegid(oldegid);

if (dir != NULL) {
free(logfile);
diff --git a/log.c b/log.c
index 8ca7d3a..6c251d2 100644
--- a/log.c
+++ b/log.c
@@ -83,14 +83,6 @@ void log_pid(t_config *config, pid_t pid) {

fprintf(fp, "%d\n", (int)pid);
fclose(fp);
-#ifndef CYGWIN
- if (chmod(config->pidfile, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH) == -1) {
- fprintf(stderr, "Warning: can't chmod PID file %s. Make sure it's only writable for root!\n", config->pidfile);
- }
- if (chown(config->pidfile, 0, 0) == -1) {
- fprintf(stderr, "Warning: can't chown PID file %s. Make sure it's owned by root!\n", config->pidfile);
- }
-#endif
}

/* Log a text.
diff --git a/monitor.c b/monitor.c
index 558f216..2768cde 100644
--- a/monitor.c
+++ b/monitor.c
@@ -90,10 +90,9 @@ static int sync_monitor_buffer(void) {
}

snprintf(filename + filename_offset, MAX_FILENAME_SIZE, "%ld.txt.gz", time(NULL));
- if ((handle = open(filename, O_CREAT | O_APPEND | O_WRONLY, 0640)) == -1) {
+ if ((handle = open(filename, O_CREAT | O_APPEND | O_WRONLY, S_IRUSR|S_IWUSR)) == -1) {
return -1;
}
- fchmod(handle, 0600);

if ((gzhandle = gzdopen(handle, "w9")) == NULL) {
close(handle);
diff --git a/serverconfig.c b/serverconfig.c
index fee42b4..8972f2a 100644
--- a/serverconfig.c
+++ b/serverconfig.c
@@ -71,7 +71,7 @@ static t_host *new_host(void) {
host->use_gz_file = false;
host->access_list = NULL;
host->alter_list = NULL;
- host->alter_fmode = 0640;
+ host->alter_fmode = S_IRUSR|S_IWUSR|S_IRGRP|S_IWGRP;
host->run_on_alter = NULL;
host->login_message = "Private page";
host->passwordfile = NULL;
diff --git a/target.c b/target.c
index 6cec305..0384e0e 100644
--- a/target.c
+++ b/target.c
@@ -498,12 +498,6 @@ int execute_cgi(t_session *session) {
return 403;
}

-#ifdef CYGWIN
- if ((session->config->platform == windows) && (session->cgi_type == binary)) {
- chmod(session->file_on_disk, 0755);
- }
-#endif
-
if ((wrap_cgi == false) && (session->cgi_type != fastcgi)) {
if (session->cgi_type == binary) {
switch (can_execute(session->file_on_disk, session->config->server_uid, session->config->server_gid, &(session->config->groups))) {
--
1.7.2.3
----------------------------------------------------------------------------------

Hiawatha version: 7.4-BuGless
Operating System: Linux
Hugo Leisink
22 January 2011, 10:45
Thanks for your contribution. I'll take a look at it soon.
This topic has been closed.