Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 21 Aug 2008 18:43:31 -0300
From:      Pedro Martelletto <pedro@ambientworks.net>
To:        freebsd-bugs@freebsd.org
Subject:   [pedro@ambientworks.net: Fix freeing of ACLs in 'setfacl']
Message-ID:  <20080821214331.GA21862@static.protection.cx>

next in thread | raw e-mail | index | archive | help
----- Forwarded message from Pedro Martelletto <pedro@ambientworks.net> -----

Date: Wed, 14 May 2008 15:48:20 -0300
From: Pedro Martelletto <pedro@ambientworks.net>
To: posix1e@FreeBSD.org
Subject: Fix freeing of ACLs in 'setfacl'

Hi,

There seems to be a double free condition in 'setfacl', as follows:

Initially, 'acl' (an 'acl_t *') is allocated, and its ACCESS_ACL and
DEFAULT_ACL fields are passed to the 'libc' ACL routines for subsequent
allocation. If the '-m' option (merge existing ACL with a new one) is
specified, then 'set_acl_mask()' will be called and passed one of the
two ACLs. This function, in turn, replaces this given ACL structure by
another, freshly allocated. However, the pointer in the 'acl' variable
in the caller is not updated. The caller then proceeds to free the ACL,
incurring in a double free condition.

This happens for every regular file, directory or symbolic link being
operated on, so the consequences are more easily visible when multiple
file system objects are involved. A proposed fix is implemented below.

Thank you for your attention,

-p.

(Please directly include my address in eventual replies, as I'm not
subscribed to this list.)

Index: setfacl.c
===================================================================
RCS file: /home/ncvs/src/bin/setfacl/setfacl.c,v
retrieving revision 1.13
diff -u -p -r1.13 setfacl.c
--- setfacl.c	26 Feb 2007 00:42:17 -0000	1.13
+++ setfacl.c	14 May 2008 18:22:18 -0000
@@ -245,10 +245,13 @@ main(int argc, char *argv[])
 			continue;
 		}
 
-		if (acl_type == ACL_TYPE_ACCESS)
+		if (acl_type == ACL_TYPE_ACCESS) {
 			final_acl = acl[ACCESS_ACL];
-		else
+			acl_free(acl[DEFAULT_ACL]);
+		} else {
 			final_acl = acl[DEFAULT_ACL];
+			acl_free(acl[ACCESS_ACL]);
+		}
 
 		if (need_mask && (set_acl_mask(&final_acl) == -1)) {
 			warnx("failed to set ACL mask on %s", file->filename);
@@ -269,8 +272,7 @@ main(int argc, char *argv[])
 			}
 		}
 
-		acl_free(acl[ACCESS_ACL]);
-		acl_free(acl[DEFAULT_ACL]);
+		acl_free(final_acl);
 		free(acl);
 	}
 

----- End forwarded message -----



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20080821214331.GA21862>