OBS User unknown 2007-10-30 00:08:18 +00:00 committed by Git OBS Bridge
parent 2d29b35d85
commit 1f4c5f5f8d
5 changed files with 312 additions and 4 deletions

View File

@ -0,0 +1,192 @@
CVE-2007-4129 coolkey file and directory permission flaw
Steve Grubb reported: "It looks like coolkey creates /tmp/.pk11ipc1 as a
world writable directory without the sticky bit. And...it creates the files
under that potentially as world writable with the execute bit turned on or
uses the file without any sanity check. coolkey runs as root sometimes and
that makes it susceptible to doing symlink attacks."
I know some folks ship coolkey here, so we've set an embargo of 20070904,
but as it's low severity are happy to extend if anyone wishes.
CVE-2007-4129 for this issue.
Proposed patch from Bob Relyea attached.
===================================================================
Index: src/coolkey/machdep.cpp
===================================================================
RCS file: /cvs/dirsec/coolkey/src/coolkey/machdep.cpp,v
retrieving revision 1.4
diff -u -r1.4 machdep.cpp
--- src/coolkey/machdep.cpp 14 Feb 2007 00:46:28 -0000 1.4
+++ src/coolkey/machdep.cpp 15 Aug 2007 01:41:11 -0000
@@ -185,12 +185,20 @@
#define MAP_INHERIT 0
#endif
+#ifndef BASEPATH
+#ifdef MAC
+#define BASEPATH "/var"
+#else
+#define BASEPATH "/var/cache"
+#endif
+#endif
+
#ifdef FULL_CLEANUP
#define RESERVED_OFFSET 256
-#define MEMSEGPATH "/tmp/.pk11ipc"
+#define MEMSEGPATH BASEPATH"/coolkey-lock"
#else
#define RESERVED_OFFSET 0
-#define MEMSEGPATH "/tmp/.pk11ipc1"
+#define MEMSEGPATH BASEPATH"/coolkey"
#endif
struct SHMemData {
@@ -208,11 +216,6 @@
#ifdef FULL_CLEANUP
flock(fd,LOCK_EX);
unsigned long ref = --(*(unsigned long *)addr);
-#ifdef notdef
- if (ref == 0) {
- unlink(path);
- }
-#endif
flock(fd, LOCK_UN);
#endif
munmap(addr,size+RESERVED_OFFSET);
@@ -225,6 +228,73 @@
}
}
+/*
+ * The cache directory is shared and accessible by anyone, make
+ * sure the cache file we are opening is really a valid cache file.
+ */
+int safe_open(char *path, int flags, int mode, int size)
+{
+ struct stat buf;
+ int fd, ret;
+
+ fd = open (path, flags|O_NOFOLLOW, mode);
+
+ if (fd < 0) {
+ return fd;
+ }
+
+ ret = fstat(fd, &buf);
+ if (ret < 0) {
+ close (fd);
+ return ret;
+ }
+
+ /* our cache files are pretty specific, make sure we are looking
+ * at the correct one */
+
+ /* first, we should own the file ourselves, don't open a file
+ * that someone else wanted us to see. */
+ if (buf.st_uid != getuid()) {
+ close(fd);
+ errno = EACCES;
+ return -1;
+ }
+
+ /* next, there should only be one link in this file. Don't
+ * use this code to trash another file */
+ if (buf.st_nlink != 1) {
+ close(fd);
+ errno = EMLINK;
+ return -1;
+ }
+
+ /* next, This better be a regular file */
+ if (!S_ISREG(buf.st_mode)) {
+ close(fd);
+ errno = EACCES;
+ return -1;
+ }
+
+ /* if the permissions don't match, something is wrong */
+ if ((buf.st_mode & 03777) != mode) {
+ close(fd);
+ errno = EACCES;
+ return -1;
+ }
+
+ /* finally the file should be the correct size. This
+ * check isn't so much to protect from an attack, as it is to
+ * detect a corrupted cache file */
+ if (buf.st_size != size) {
+ close(fd);
+ errno = EACCES;
+ return -1;
+ }
+
+ /* OK, the file checked out, ok to continue */
+ return fd;
+}
+
SHMem::SHMem(): shmemData(0) {}
SHMem *
@@ -248,7 +318,7 @@
return NULL;
}
int mask = umask(0);
- int ret = mkdir (MEMSEGPATH, 0777);
+ int ret = mkdir (MEMSEGPATH, 1777);
umask(mask);
if ((ret == -1) && (errno != EEXIST)) {
delete shmemData;
@@ -264,21 +334,16 @@
shmemData->path[sizeof(MEMSEGPATH)-1] = '/';
strcpy(&shmemData->path[sizeof(MEMSEGPATH)],name);
- int mode = 0777;
- if (strcmp(name,"token_names") != 0) {
- /* each user gets his own uid array */
- sprintf(uid_str, "-%u",getuid());
- strcat(shmemData->path,uid_str);
- mode = 0700;
- }
+ sprintf(uid_str, "-%u",getuid());
+ strcat(shmemData->path,uid_str);
+ int mode = 0600;
+
shmemData->fd = open(shmemData->path,
O_CREAT|O_RDWR|O_EXCL|O_APPEND|O_EXLOCK, mode);
- if (shmemData->fd < 0) {
- needInit = false;
- shmemData->fd = open(shmemData->path,O_RDWR|O_EXLOCK, mode);
- } else {
+ if (shmemData->fd >= 0) {
char *buf;
int len = size+RESERVED_OFFSET;
+ int ret;
buf = (char *)calloc(1,len);
if (!buf) {
@@ -289,8 +354,22 @@
delete shmemData;
return NULL;
}
- write(shmemData->fd,buf,len);
+ ret = write(shmemData->fd,buf,len);
+ if (ret != len) {
+ unlink(shmemData->path);
+#ifdef FULL_CLEANUP
+ flock(shmemData->fd, LOCK_UN);
+#endif
+ delete shmemData;
+ return NULL;
+ }
+
free(buf);
+ } else if (errno == EEXIST) {
+ needInit = false;
+
+ shmemData->fd = safe_open(shmemData->path,O_RDWR|O_EXLOCK, mode,
+ size+RESERVED_OFFSET);
}
if (shmemData->fd < 0) {
delete shmemData;

View File

@ -0,0 +1,73 @@
https://bugzilla.redhat.com/show_bug.cgi?id=356971
In file included from object.cpp:22:
object.h:94: warning: type qualifiers ignored on function return type
object.cpp: In member function 'void PKCS11Object::getAttributeValue(CK_ATTRIBUTE*, CK_ULONG, Log*) const':
object.cpp:373: error: 'memcpy' was not declared in this scope
object.cpp: In member function 'const char* PKCS11Object::getLabel()':
object.cpp:417: error: 'memcpy' was not declared in this scope
object.cpp: In member function 'CK_OBJECT_CLASS PKCS11Object::getClass()':
object.cpp:442: error: 'memcpy' was not declared in this scope
object.cpp: In member function 'void PKCS11Object::setAttribute(CK_ATTRIBUTE_TYPE, const char*)':
object.cpp:465: error: 'strlen' was not declared in this scope
object.cpp: In function 'SECStatus GetCN(const CKYByte*, unsigned int, CCItem*)':
object.cpp:979: error: 'memcmp' was not declared in this scope
object.cpp: In function 'char* GetUserName(const CKYBuffer*)':
object.cpp:1010: error: 'memcpy' was not declared in this scope
machdep.cpp: In static member function 'static SHMem* SHMem::initSegment(const char*, int, bool&)':
machdep.cpp:328: error: 'strlen' was not declared in this scope
machdep.cpp:333: error: 'memcpy' was not declared in this scope
machdep.cpp:335: error: 'strcpy' was not declared in this scope
machdep.cpp:338: error: 'strcat' was not declared in this scope
machdep.cpp:348: error: 'calloc' was not declared in this scope
machdep.cpp:367: error: 'free' was not declared in this scope
log.cpp: In member function 'virtual void SysLog::log(const char*, ...)':
log.cpp:100: error: 'strlen' was not declared in this scope
log.cpp:100: error: 'malloc' was not declared in this scope
log.cpp:102: error: 'strcpy' was not declared in this scope
log.cpp:103: error: 'strcat' was not declared in this scope
log.cpp:106: error: 'free' was not declared in this scope
slot.cpp:36: error: 'std::auto_ptr' has not been declared
================================================================================
--- src/coolkey/log.cpp
+++ src/coolkey/log.cpp
@@ -21,6 +21,8 @@
#include "mypkcs11.h"
#include <assert.h>
#include <stdio.h>
+#include <string.h>
+#include <stdlib.h>
#include "log.h"
#include <cstdarg>
#include "PKCS11Exception.h"
--- src/coolkey/machdep.cpp
+++ src/coolkey/machdep.cpp
@@ -27,6 +27,8 @@
#include <unistd.h>
#include <errno.h>
#include <fcntl.h>
+#include <string.h>
+#include <stdlib.h>
#include <sys/file.h>
#include <sys/types.h>
#include <sys/time.h>
--- src/coolkey/object.cpp
+++ src/coolkey/object.cpp
@@ -21,6 +21,8 @@
#include "PKCS11Exception.h"
#include "object.h"
#include <algorithm>
+#include <string.h>
+#include <stdlib.h>
using std::find_if;
--- src/coolkey/slot.cpp
+++ src/coolkey/slot.cpp
@@ -18,6 +18,7 @@
* ***** END COPYRIGHT BLOCK *****/
#include <string>
+#include <memory>
#include "mypkcs11.h"
#include <stdio.h>
#include <assert.h>

22
coolkey-null.patch Normal file
View File

@ -0,0 +1,22 @@
https://bugzilla.redhat.com/show_bug.cgi?id=356971
coolkey.cpp:37:1: error: "NULL" redefined
In file included from /usr/include/alloca.h:25,
from /usr/include/stdlib.h:612,
from /usr/include/c++/4.3.0/cstdlib:73,
from /usr/include/c++/4.3.0/bits/stl_algo.h:65,
from /usr/include/c++/4.3.0/algorithm:67,
from slot.h:27,
from coolkey.cpp:33:
/usr/lib64/gcc/x86_64-suse-linux/4.3.0/include/stddef.h:400:1: error: this is the location of the previous definition
================================================================================
--- src/coolkey/coolkey.cpp
+++ src/coolkey/coolkey.cpp
@@ -34,8 +34,6 @@
#include "cky_base.h"
#include "params.h"
-#define NULL 0
-
/* static module data -------------------------------- */
static Log *log = NULL;

View File

@ -1,3 +1,14 @@
-------------------------------------------------------------------
Mon Oct 29 17:50:46 CET 2007 - sbrabec@suse.cz
- Fixed gcc 4.3 build errors.
-------------------------------------------------------------------
Mon Sep 10 13:28:16 CEST 2007 - sbrabec@suse.cz
- Fixed file and directory permission flaw (#304180,
CVE-2007-4129).
------------------------------------------------------------------- -------------------------------------------------------------------
Thu Sep 6 21:03:20 CEST 2007 - jberkman@novell.com Thu Sep 6 21:03:20 CEST 2007 - jberkman@novell.com

View File

@ -11,16 +11,19 @@
Name: coolkey Name: coolkey
Version: 1.1.0 Version: 1.1.0
Release: 10 Release: 22
Summary: CoolKey PKCS #11 PKI Module for Smart Cards Summary: CoolKey PKCS #11 PKI Module for Smart Cards
License: LGPL v2 only License: LGPL v2.1 only
Group: Productivity/Security Group: Productivity/Security
URL: http://directory.fedoraproject.org/wiki/CoolKey Url: http://directory.fedoraproject.org/wiki/CoolKey
Source: %{name}-%{version}.tar.bz2 Source: %{name}-%{version}.tar.bz2
Patch: coolkey-configure-syntax-error.patch Patch: coolkey-configure-syntax-error.patch
Patch1: coolkey-string-literal-comparison.patch Patch1: coolkey-string-literal-comparison.patch
Patch2: coolkey-amflags.patch Patch2: coolkey-amflags.patch
Patch3: coolkey-1.1.0-evoandooo.patch Patch3: coolkey-1.1.0-evoandooo.patch
Patch4: coolkey-cache-dir-move.patch
Patch5: coolkey-null.patch
Patch6: coolkey-implicit-declaration.patch
BuildRoot: %{_tmppath}/%{name}-%{version}-build BuildRoot: %{_tmppath}/%{name}-%{version}-build
BuildRequires: gcc-c++ mozilla-nss-devel pcsc-lite-devel pkg-config zlib-devel BuildRequires: gcc-c++ mozilla-nss-devel pcsc-lite-devel pkg-config zlib-devel
#Requires: pcsc-lite #Requires: pcsc-lite
@ -83,6 +86,9 @@ card and USB Fob form factors.
%patch1 %patch1
%patch2 %patch2
%patch3 -p1 %patch3 -p1
%patch4
%patch5
%patch6
%build %build
autoreconf -f -i autoreconf -f -i
@ -119,8 +125,12 @@ rm -rf $RPM_BUILD_ROOT
%{_libdir}/libckyapplet.so %{_libdir}/libckyapplet.so
%{_libdir}/pkgconfig/*.pc %{_libdir}/pkgconfig/*.pc
%{_includedir}/*.h %{_includedir}/*.h
%changelog %changelog
* Mon Oct 29 2007 - sbrabec@suse.cz
- Fixed gcc 4.3 build errors.
* Mon Sep 10 2007 - sbrabec@suse.cz
- Fixed file and directory permission flaw (#304180,
CVE-2007-4129).
* Thu Sep 06 2007 - jberkman@novell.com * Thu Sep 06 2007 - jberkman@novell.com
- install pk11install - install pk11install
- teach pk11install about evolution and openoffice - teach pk11install about evolution and openoffice