Hi,
On Wed, Nov 17, 2010 at 09:12:26PM +0100, Moritz Muehlenhoff wrote:
> Package: libsdp
> Severity: grave
> Tags: security
>
> Please see https://bugzilla.redhat.com/show_bug.cgi?id=647941
> for details.
>
> Please fix this in unstable with an isolated fix and asking
> release managers for an unblock afterwards.
I'm attaching the isolated upstream fix, please test and take of
an upload.
Cheers,
Moritz
>From c6efc06bf6123ad3a731b672bf90b33e630c7bf6 Mon Sep 17 00:00:00 2001
From: Amir Vadai <am...@mellanox.co.il>
Date: Mon, 8 Nov 2010 14:31:13 +0200
Subject: [PATCH] libsdp: fix security issues with log file
When run as regular user, must make sure logs file
is not a symbolic link or with bad owner/permissions.
Signed-off-by: Amir Vadai <am...@mellanox.co.il>
---
libsdp.conf | 16 ++++++++++------
src/log.c | 34 ++++++++++++++++++++++++++++++----
2 files changed, 40 insertions(+), 10 deletions(-)
diff --git a/libsdp.conf b/libsdp.conf
index 5b03e13..65f1f83 100644
--- a/libsdp.conf
+++ b/libsdp.conf
@@ -16,8 +16,8 @@
# Please do not forget to comment if you want to change these.
# (the rest of this file explains the syntax and give examples)
#
-# Get errors printed into the files /tmp/libsdp.log.<uid>
-# or /var/log/<filename> for root
+# Get errors printed into the files /tmp/libsdp.log.<uid>/log
+# or /var/log/<path> for root
log min-level 9 destination file libsdp.log
#
# By default we let all servers and client try SDP first.
@@ -32,14 +32,18 @@ use both client * *:*
# ------------------
# The log directive allows the user to specify which and where debug and error
# messages get sent. The log statement format is:
-# log [destination stderr|syslog|file <filename>] [min-level <1-9>]
+# log [destination stderr|syslog|file <path>] [min-level <1-9>]
#
# destination - defines the destination of the log messages:
# stderr - messages will be forwarded to the stderr
# syslog - messages sent to the syslog service
-# file <filename> - messages will be written to the file /var/log/<filename> for root.
-# for regular user, if full path is requsted <filename with path>.<uid>
-# or /tmp/<filename>.<uid> if no path is requested
+# file <path> - messages will be written to the file /var/log/<path> for root.
+# for regular user, if full path is requsted <path>.<uid>/log
+# or /tmp/<path>.<uid>/log if no path is requested
+# Due to security reasons, <path> must not be:
+# 1. a soft link
+# 2. owned by other user.
+# 3. Other permissions except User permissions.
#
# min-level - defines the verbosity of the log:
# 9 - only errors are printed
diff --git a/src/log.c b/src/log.c
index 9b62bc3..c98ee46 100644
--- a/src/log.c
+++ b/src/log.c
@@ -176,17 +176,43 @@ __sdp_log_set_log_file(
if (uid == 0) {
if ( p )
filename = p + 1;
- snprintf( tfilename, sizeof( tfilename ), "/var/log/%s", filename );
+ snprintf( tfilename, sizeof(tfilename), "/var/log/%s", filename );
} else {
+ char tdir[PATH_MAX + 1];
/*
for regular user, allow log file to be placed in a user
requested path. If no path is requested the log file is
placed in /tmp/
*/
if ( p )
- snprintf( tfilename, sizeof( tfilename ), "%s.%d", filename, uid );
+ snprintf(tdir, sizeof(tdir), "%s.%d", filename, uid );
else
- snprintf( tfilename, sizeof( tfilename ), "/tmp/%s.%d", filename, uid );
+ snprintf(tdir, sizeof(tdir ), "/tmp/%s.%d", filename, uid );
+
+ if (mkdir(tdir, 0700)) {
+ struct stat stat;
+
+ if (errno != EEXIST) {
+ __sdp_log( 9, "Couldn't create directory '%s' for logging (%m)\n", tdir );
+ return 0;
+ }
+
+ if (lstat(tdir, &stat)) {
+ __sdp_log(9, "Couldn't lstat directory %s\n", tdir);
+ return 0;
+ }
+
+ if (!S_ISDIR(stat.st_mode) || stat.st_uid != uid ||
+ (stat.st_mode & ~(S_IFMT | S_IRWXU))) {
+ __sdp_log( 9, "Cowardly refusing to log into directory:'%s'. "
+ "Make sure it is not: (1) link, (2) other uid, (3) bad permissions."
+ "thus is a security issue.\n", tdir );
+ return 0;
+ }
+ }
+
+ snprintf(tfilename, sizeof(tfilename), "%s/log", tdir);
+ printf("dir: %s file: %s\n", tdir, tfilename);
}
/* double check the file is not a link */
@@ -199,7 +225,7 @@ __sdp_log_set_log_file(
f = fopen( tfilename, "a" );
if ( !f ) {
- __sdp_log( 9, "Couldn't open filename '%s' for logging\n", tfilename );
+ __sdp_log( 9, "Couldn't open '%s' for logging (%m)\n", tfilename );
return 0;
}
--
1.7.0.4
