From owner-freebsd-bugs@FreeBSD.ORG Thu Jun 26 14:10:04 2008 Return-Path: Delivered-To: freebsd-bugs@hub.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 9D8AC106567A for ; Thu, 26 Jun 2008 14:10:04 +0000 (UTC) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (freefall.freebsd.org [IPv6:2001:4f8:fff6::28]) by mx1.freebsd.org (Postfix) with ESMTP id 70E6E8FC19 for ; Thu, 26 Jun 2008 14:10:04 +0000 (UTC) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (gnats@localhost [127.0.0.1]) by freefall.freebsd.org (8.14.2/8.14.2) with ESMTP id m5QEA4bi048116 for ; Thu, 26 Jun 2008 14:10:04 GMT (envelope-from gnats@freefall.freebsd.org) Received: (from gnats@localhost) by freefall.freebsd.org (8.14.2/8.14.1/Submit) id m5QEA4DC048115; Thu, 26 Jun 2008 14:10:04 GMT (envelope-from gnats) Resent-Date: Thu, 26 Jun 2008 14:10:04 GMT Resent-Message-Id: <200806261410.m5QEA4DC048115@freefall.freebsd.org> Resent-From: FreeBSD-gnats-submit@FreeBSD.org (GNATS Filer) Resent-To: freebsd-bugs@FreeBSD.org Resent-Reply-To: FreeBSD-gnats-submit@FreeBSD.org, clemens fischer Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id E3BFE106567C for ; Thu, 26 Jun 2008 14:07:46 +0000 (UTC) (envelope-from ino-news@kabelmail.de) Received: from smtpa.mediabeam.com (smtpa1.mediabeam.com [194.25.41.13]) by mx1.freebsd.org (Postfix) with ESMTP id 351158FC23 for ; Thu, 26 Jun 2008 14:07:45 +0000 (UTC) (envelope-from ino-news@kabelmail.de) Received: from spotteswoode.dnsalias.org (91-64-207-64-dynip.superkabel.de [91.64.207.64]) (authenticated bits=0) by smtpa.mediabeam.com (8.13.0/8.13.1) with ESMTP id m5QDuIot029411 for ; Thu, 26 Jun 2008 15:56:18 +0200 Received: by spotteswoode.dnsalias.org (Postfix, from userid 0) id D669F371D8; Thu, 26 Jun 2008 15:55:49 +0200 (CEST) Message-Id: <20080626135549.D669F371D8@spotteswoode.dnsalias.org> Date: Thu, 26 Jun 2008 15:55:49 +0200 (CEST) From: clemens fischer To: FreeBSD-gnats-submit@FreeBSD.org X-Send-Pr-Version: 3.113 Cc: Subject: kern/125009: access(2) grants root execute perms for non-executable files X-BeenThere: freebsd-bugs@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: clemens fischer List-Id: Bug reports List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 26 Jun 2008 14:10:04 -0000 >Number: 125009 >Category: kern >Synopsis: access(2) grants root execute perms for non-executable files >Confidential: no >Severity: serious >Priority: medium >Responsible: freebsd-bugs >State: open >Quarter: >Keywords: >Date-Required: >Class: sw-bug >Submitter-Id: current-users >Arrival-Date: Thu Jun 26 14:10:04 UTC 2008 >Closed-Date: >Last-Modified: >Originator: clemens fischer >Release: FreeBSD 8.0-CURRENT i386 >Organization: >Environment: System: FreeBSD spotteswoode.dnsalias.org 8.0-CURRENT FreeBSD 8.0-CURRENT #21: Mon Jun 23 20:24:05 CEST 2008 root@spotteswoode.dnsalias.org:/usr/obj/usr/src/sys/spott_fbsd8_i386 i386 >Description: i found the problem when trying out "git" (the VCS). per default, git-init(1) creates repos containing eg. pre-commit hooks, shell scripts to verify commits etc. they are mode 0644 or mode 0664 and can be changed by the user. to enable them, one has to "chmod +x" them. unprivileged users can use (newer) git-releases without worrying about this, but if root wants to operate a git repo, every commit aborts with messages like: "fatal: exec .../.git/hooks/prepare-commit-msg failed." for every hook involved. after "ktrace git-commit", kdump(1) shows: : 73042 git-commit CALL access(0x8123320,X_OK) : 73042 git-commit NAMI "/home/src/bulk/ion/.git/hooks/prepare-commit-msg" : 73042 git-commit RET access 0 : 73042 git-commit CALL fork : 73042 git-commit RET fork 73043/0x11d53 : 73043 git-commit RET fork 0 : 73043 git-commit CALL open(0x80f1b83,O_RDWR,0) : 73043 git-commit NAMI "/dev/null" : 73042 git-commit CALL wait4(0x11d53,0xbfbfd2cc,0,0) : 73043 git-commit RET open 3 : 73043 git-commit CALL dup2(0x3,0) : 73043 git-commit RET dup2 0 : 73043 git-commit CALL close(0x3) : 73043 git-commit RET close 0 : 73043 git-commit CALL dup2(0x2,0x1) : 73043 git-commit RET dup2 1 : 73043 git-commit CALL execve(0x8123320,0xbfbfd728,0x8213400) : 73043 git-commit NAMI "/home/src/bulk/ion/.git/hooks/prepare-commit-msg" : 73043 git-commit RET execve -1 errno 13 Permission denied : 73043 git-commit CALL stat(0x8123320,0xbfbfcdf8) : 73043 git-commit NAMI "/home/src/bulk/ion/.git/hooks/prepare-commit-msg" : 73043 git-commit STRU struct stat {dev=117, ino=800809, mode=-rw-r--r-- , nlink=1, uid=0, gid=0, rdev=3262495, atime=1214332267, stime=1214331402, ctime=1214331402, birthtime=1214331402, size=1196, blksize=4096, blocks=4, flags=0x0 } : 73043 git-commit RET stat 0 : 73043 git-commit CALL write(0x2,0xbfbfca38,0x45) : 73043 git-commit GIO fd 2 wrote 69 bytes : "fatal: exec /home/src/bulk/ion/.git/hooks/prepare-commit-msg failed. : " i verified this in the current git source: in git/builtin-commit.c::run_hook() there's the sequence: : if (access(argv[0], X_OK) < 0) : return 0; : : memset(&hook, 0, sizeof(hook)); : hook.argv = argv; : hook.no_stdin = 1; : hook.stdout_to_stderr = 1; : hook.env = env; : : return run_command(&hook); this check works correctly for non-root, but any file accessible in any way for root _passes_ the "(access(argv[0], X_OK) < 0)" check, a subsequent execvp(3) fails. doing "git-init; date > fileA; git-commit -a" as a non-privileged user works as expected. but this isn't a git-problem anyway: i took /usr/src/tools/regression/security/access/* and changed every check for "access(path, R_OK)" into "access(path, X_OK)". as the test files in this test are created modes 0400 and 0040, one would expect every check to fail when using X_OK, but the output is: : /src/localcode/test/access : 0 # ./testaccess : Effective uid was used instead of real uid in access(). : Effective gid was used instead of real gid in access(). : 2 errors seen. >How-To-Repeat: you can either install ports/devel/git and do: : git-init : date > fileA : git-add fileA : git-commit as 1) root and 2) somebody_else and watch the difference: : (1) "fatal: exec /tmp/.git/hooks/pre-commit failed." : (2) "Created initial commit..." -or- you could apply the following patch in /usr/src/tools/regression/security/access/, and do "make; ./testaccess": : /src/localcode/test/access : 0 # ./testaccess : Effective uid was used instead of real uid in access(). : Effective gid was used instead of real gid in access(). : 2 errors seen. i'd expect each and every test to fail as none of the test files is executable! pseudo-patch replacing R_OK with X_OK (only for testing!): : /src/localcode/test/access : 0 # hg diff : diff --git a/access/testaccess.c b/access/testaccess.c : --- a/access/testaccess.c : +++ b/access/testaccess.c : @@ -216,14 +216,14 @@ : : errorseen = 0; : : - error = access("test1", R_OK); : + error = access("test1", /*R_OK*/ X_OK); : if (!error) { : fprintf(stderr, "saved uid used instead of real uid\n"); : errorseen++; : } : : #ifdef EACCESS_AVAILABLE : - error = eaccess("test1", R_OK); : + error = eaccess("test1", /*R_OK*/ X_OK); : if (!error) { : fprintf(stderr, "saved uid used instead of effective uid\n"); : errorseen++; : @@ -245,7 +245,7 @@ : } : : /* Check that the real uid is used, not the effective uid */ : - error = access("test2", R_OK); : + error = access("test2", /*R_OK*/ X_OK); : if (error) { : fprintf(stderr, "Effective uid was used instead of real uid in access().\n"); : errorseen++; : @@ -253,7 +253,7 @@ : : #ifdef EACCESS_AVAILABLE : /* Check that the effective uid is used, not the real uid */ : - error = eaccess("test3", R_OK); : + error = eaccess("test3", /*R_OK*/ X_OK); : if (error) { : fprintf(stderr, "Real uid was used instead of effective uid in eaccess().\n"); : errorseen++; : @@ -261,7 +261,7 @@ : #endif : : /* Check that the real uid is used, not the effective uid */ : - error = access("test3", R_OK); : + error = access("test3", /*R_OK*/ X_OK); : if (!error) { : fprintf(stderr, "Effective uid was used instead of real uid in access().\n"); : errorseen++; : @@ -269,7 +269,7 @@ : : #ifdef EACCESS_AVAILABLE : /* Check that the effective uid is used, not the real uid */ : - error = eaccess("test2", R_OK); : + error = eaccess("test2", /*R_OK*/ X_OK); : if (!error) { : fprintf(stderr, "Real uid was used instead of effective uid in eaccess().\n"); : errorseen++; : @@ -299,13 +299,13 @@ : } : : /* Check that the saved gid is not used */ : - error = access("test4", R_OK); : + error = access("test4", /*R_OK*/ X_OK); : if (!error) { : fprintf(stderr, "saved gid used instead of real gid\n"); : } : : #ifdef EACCESS_AVAILABLE : - error = eaccess("test4", R_OK); : + error = eaccess("test4", /*R_OK*/ X_OK); : if (!error) { : fprintf(stderr, "saved gid used instead of effective gid\n"); : errorseen++; : @@ -313,7 +313,7 @@ : #endif : : /* Check that the real gid is used, not the effective gid */ : - error = access("test5", R_OK); : + error = access("test5", /*R_OK*/ X_OK); : if (error) { : fprintf(stderr, "Effective gid was used instead of real gid in access().\n"); : errorseen++; : @@ -321,7 +321,7 @@ : : #ifdef EACCESS_AVAILABLE : /* Check that the effective gid is used, not the real gid */ : - error = eaccess("test6", R_OK); : + error = eaccess("test6", /*R_OK*/ X_OK); : if (error) { : fprintf(stderr, "Real gid was used instead of effective gid in eaccess().\n"); : errorseen++; : @@ -329,7 +329,7 @@ : #endif : : /* Check that the real gid is used, not the effective gid */ : - error = access("test6", R_OK); : + error = access("test6", /*R_OK*/ X_OK); : if (!error) { : fprintf(stderr, "Effective gid was used instead of real gid in access().\n"); : errorseen++; : @@ -337,7 +337,7 @@ : : #ifdef EACCESS_AVAILABLE : /* Check that the effective gid is used, not the real gid */ : - error = eaccess("test5", R_OK); : + error = eaccess("test5", /*R_OK*/ X_OK); : if (!error) { : fprintf(stderr, "Real gid was used instead of effective gid in eaccess().\n"); : errorseen++; >Fix: >Release-Note: >Audit-Trail: >Unformatted: