[prev in list] [next in list] [prev in thread] [next in thread]
List: openjdk-serviceability-dev
Subject: Don't check gids of files against primary gid of java process
From: martinrb () google ! com (Martin Buchholz)
Date: 2009-10-28 23:35:47
Message-ID: 1ccfd1c10910281635l5e73c4c9tbf2d1e554d6a4a84 () mail ! gmail ! com
[Download RAW message or body]
Hi serviceability/ptrace/attach team,
This is a bug report with fix.
This involves changes to both hotspot and jdk repos,
and to platform-specific code that is completely untested,
which makes it particularly difficult to integrate,
so I'm hoping a serviceability engineer at Sun will
take over that task from me.
More work than I have done could be done
on generating better error messages, i.e. avoid
"well-known file is not secure" (huh?)
This allows various java APIs to work much better
when setgid bits are used on directories.
We don't believe this introduces a security bug,
but the reviewers should verify that for yourself.
Don't check gids of files against primary gid of java process,
since this will cause failure when files are written to
directories with the setgid bit on. Also, make error
reporting more informative.
Failure can be reproduced as follows:
(JDK="...."; cat $cdtoy/Sleeper.java; rm -rf /tmp/t7 && mkdir
/tmp/t7 && chgrp guest /tmp/t7 && chmod g+s /tmp/t7 && CHDIR /tmp/t7
&& $JDK/bin/java -XX:+StartAttachListener -cp $cdtoy Sleeper & sleep 3
&& $JDK/bin/jmap -histo $(jps | awk '/Sleeper/ { print $1; }'); kill
%1)
public class Sleeper {
public static void main(String[] args) throws Throwable {
Runtime.getRuntime().exec(new String[]
{"/bin/sleep", "120"}).waitFor();}}
6153: well-known file is not secure
--- hotspot/agent/src/os/linux/ps_proc.c 2009-10-28 15:51:29.000000000 -0700
+++ hotspot/agent/src/os/linux/ps_proc.c 2009-10-28 16:32:10.000000000 -0700
@@ -147,7 +147,8 @@
// attach to a process/thread specified by "pid"
static bool ptrace_attach(pid_t pid) {
if (ptrace(PTRACE_ATTACH, pid, NULL, NULL) < 0) {
- print_debug("ptrace(PTRACE_ATTACH, ..) failed for %d\n", pid);
+ print_debug("ptrace(PTRACE_ATTACH, %d, ..) failed: %s\n",
+ pid, strerror(errno));
return false;
} else {
int ret;
@@ -270,7 +271,8 @@
// detach a given pid
static bool ptrace_detach(pid_t pid) {
if (pid && ptrace(PTRACE_DETACH, pid, NULL, NULL) < 0) {
- print_debug("ptrace(PTRACE_DETACH, ..) failed for %d\n", pid);
+ print_debug("ptrace(PTRACE_DETACH, %d, ..) failed: %s\n",
+ pid, strerror(errno));
return false;
} else {
return true;
--- hotspot/src/os/solaris/dtrace/jvm_dtrace.c 2009-10-28
15:51:29.000000000 -0700
+++ hotspot/src/os/solaris/dtrace/jvm_dtrace.c 2009-10-28
16:32:10.000000000 -0700
@@ -143,25 +143,21 @@
/* called to check permissions on attach file */
static int check_permission(const char* path) {
struct stat64 sb;
- uid_t uid, gid;
- int res;
/*
- * Check that the path is owned by the effective uid/gid of this
+ * Check that the path is owned by the effective uid of this
* process. Also check that group/other access is not allowed.
*/
- uid = geteuid();
- gid = getegid();
-
- res = stat64(path, &sb);
- if (res != 0) {
- print_debug("stat failed for %s\n", path);
+ if (stat64(path, &sb) != 0) {
+ print_debug("stat failed for %s: %s\n", path, strerror(errno));
return -1;
}
- if ((sb.st_uid != uid) || (sb.st_gid != gid) ||
+ if ((sb.st_uid != geteuid()) ||
((sb.st_mode & (S_IRGRP|S_IWGRP|S_IROTH|S_IWOTH)) != 0)) {
print_debug("well-known file %s is not secure\n", path);
+ print_debug("process uid=%ld file uid=%ld mode=%lo\n",
+ (long) geteuid(), (long) sb_st_uid, (long) sb.st_mode);
return -1;
}
return 0;
--- jdk/src/solaris/native/sun/tools/attach/LinuxVirtualMachine.c 2009-10-28
15:51:29.000000000 -0700
+++ jdk/src/solaris/native/sun/tools/attach/LinuxVirtualMachine.c 2009-10-28
16:32:10.000000000 -0700
@@ -351,39 +351,42 @@
const char* p = GetStringPlatformChars(env, path, &isCopy);
if (p != NULL) {
struct stat64 sb;
- uid_t uid, gid;
- int res;
/*
- * Check that the path is owned by the effective uid/gid of this
+ * Check that the path is owned by the effective uid of this
* process. Also check that group/other access is not allowed.
*/
- uid = geteuid();
- gid = getegid();
-
- res = stat64(p, &sb);
- if (res != 0) {
- /* save errno */
- res = errno;
- }
-
- /* release p here before we throw an I/O exception */
- if (isCopy) {
- JNU_ReleaseStringPlatformChars(env, path, p);
- }
-
- if (res == 0) {
- if ( (sb.st_uid != uid) || (sb.st_gid != gid) ||
+ if (stat64(p, &sb) == 0) {
+ if ( (sb.st_uid != geteuid()) ||
((sb.st_mode & (S_IRGRP|S_IWGRP|S_IROTH|S_IWOTH)) != 0) ) {
- JNU_ThrowIOException(env, "well-known file is not secure");
+ const char* format =
+ "well-known file %s is not secure:"
+ " process uid=%ld file uid=%ld mode=%lo\n";
+ const int MAX_INT_LENGTH = 50;
+ char *msg = malloc(strlen(format) +
+ strlen(p) +
+ 3 * MAX_INT_LENGTH);
+ if (msg != NULL) {
+ sprintf(msg, format,
+ p,
+ (long) geteuid(),
+ (long) sb.st_uid,
+ (long) sb.st_mode);
+ JNU_ThrowIOException(env, msg);
+ free(msg);
+ }
}
} else {
- char* msg = strdup(strerror(res));
+ char* msg = strdup(strerror(errno));
JNU_ThrowIOException(env, msg);
if (msg != NULL) {
free(msg);
}
}
+
+ if (isCopy) {
+ JNU_ReleaseStringPlatformChars(env, path, p);
+ }
}
}
--- jdk/src/solaris/native/sun/tools/attach/SolarisVirtualMachine.c 2009-10-28
15:51:29.000000000 -0700
+++ jdk/src/solaris/native/sun/tools/attach/SolarisVirtualMachine.c 2009-10-28
16:32:10.000000000 -0700
@@ -96,39 +96,42 @@
const char* p = GetStringPlatformChars(env, path, &isCopy);
if (p != NULL) {
struct stat64 sb;
- uid_t uid, gid;
- int res;
/*
- * Check that the path is owned by the effective uid/gid of this
+ * Check that the path is owned by the effective uid of this
* process. Also check that group/other access is not allowed.
*/
- uid = geteuid();
- gid = getegid();
-
- res = stat64(p, &sb);
- if (res != 0) {
- /* save errno */
- res = errno;
- }
-
- /* release p here before we throw an I/O exception */
- if (isCopy) {
- JNU_ReleaseStringPlatformChars(env, path, p);
- }
-
- if (res == 0) {
- if ( (sb.st_uid != uid) || (sb.st_gid != gid) ||
+ if (stat64(p, &sb) == 0) {
+ if ( (sb.st_uid != geteuid()) ||
((sb.st_mode & (S_IRGRP|S_IWGRP|S_IROTH|S_IWOTH)) != 0) ) {
- JNU_ThrowIOException(env, "well-known file is not secure");
+ const char* format =
+ "well-known file %s is not secure:"
+ " process uid=%ld file uid=%ld mode=%lo\n";
+ const int MAX_INT_LENGTH = 50;
+ char *msg = malloc(strlen(format) +
+ strlen(p) +
+ 3 * MAX_INT_LENGTH);
+ if (msg != NULL) {
+ sprintf(msg, format,
+ p,
+ (long) geteuid(),
+ (long) sb.st_uid,
+ (long) sb.st_mode);
+ JNU_ThrowIOException(env, msg);
+ free(msg);
+ }
}
} else {
- char* msg = strdup(strerror(res));
+ char* msg = strdup(strerror(errno));
JNU_ThrowIOException(env, msg);
if (msg != NULL) {
free(msg);
}
}
+
+ if (isCopy) {
+ JNU_ReleaseStringPlatformChars(env, path, p);
+ }
}
}
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic