commit 6164445cceb24fa691595d856c763e278428e68f
Author: Michael Forney <mforney_AT_mforney.org>
Date:   Sun Nov 2 22:24:52 2014 +0000
    Fix symbolic mode parsing in parsemode
    
    I found quite a lot of bugs, so I ended up pretty much rewriting as I
    followed the spec¹.
    
    Now, it works as follows:
    
    - Determine a mask (who) of bits that can be modified for the subsequent
      operations. If none are specified, select all file mode bits.
    - In a loop, determine which operation (+, -, =) to apply.
    - If the next character is a permcopy (u, g, o), set the new permissions
      (perm) corresponding to the bits set in the user, group or other parts
      of the existing mode.
    - Otherwise, set the new permissions by looping through the next r, w,
      x, s, t characters.
    - Now, the set of bits we want to add or remove is (who & perm). Set or
      remove these bits according the the operation (first clearing the
      appropriate bits if the operation is =).
    - Continue from the top if the next character is a comma, otherwise,
      process the next operation.
    
    I tested this on some made up inputs, and I believe it is working
    correctly now:
    
      parsemode("g+w", 0644, 0),    before: 0606, now: 0664, fixed
      parsemode("u+rx", 0222, 0),   before: 0077, now: 0722, fixed
      parsemode("+x", 0644, 023),   before: 0754, now: 0754, still works
      parsemode("+w", 0444, 022),   before: 0644, now: 0644, still works
      parsemode("+w", 0444, 0),     before: 0666, now: 0666, still works
      parsemode("+s", 0755, 0),     before: 0755, now: 6755, fixed
      parsemode("u+s", 0755, 0),    before: 0055, now: 4755, fixed
      parsemode("g+s", 0755, 0),    before: 0705, now: 2755, fixed
      parsemode("g=u", 0700, 0),    before: 0000, now: 0770, fixed
      parsemode("go=u-w", 0700, 0), before: 0000, now: 0755, fixed
      parsemode("o+u-g", 0641, 0),  before: 0000, now: 0643, fixed
      parsemode("u=rx,o+w,g-r", 0654, 0) before: error, now: 0516, fixed
      parsemode(",", 0654, 0), before: error, now: error, still works
    
    ¹ 
http://pubs.opengroup.org/onlinepubs/9699919799/utilities/chmod.html
diff --git a/util/mode.c b/util/mode.c
index 4506023..c022e8d 100644
--- a/util/mode.c
+++ b/util/mode.c
_AT_@ -16,9 +16,9 @@ mode_t
 parsemode(const char *str, mode_t mode, mode_t mask)
 {
         char *end;
-	const char *p;
-	int octal, op = '+';
-	mode_t gmask = 0, m = 0;
+	const char *p = str;
+	int octal, op;
+	mode_t who, perm, clear;
 
         octal = strtol(str, &end, 8);
         if(*end == '\0') {
_AT_@ -41,61 +41,122 @@ parsemode(const char *str, mode_t mode, mode_t mask)
                 if(octal & 00001) mode |= S_IXOTH;
                 return mode;
         }
-	for(p = str; *p; p++) {
+next:
+	/* first, determine which bits we will be modifying */
+	for(who = 0; *p; p++) {
                 switch(*p) {
                 /* masks */
                 case 'u':
-			gmask |= S_IRWXU;
-			break;
+			who |= S_IRWXU|S_ISUID;
+			continue;
                 case 'g':
-			gmask |= S_IRWXG;
-			break;
+			who |= S_IRWXG|S_ISGID;
+			continue;
                 case 'o':
-			gmask |= S_IRWXO;
-			break;
+			who |= S_IRWXO;
+			continue;
                 case 'a':
-			gmask |= S_IRWXU|S_IRWXG|S_IRWXO;
-			break;
+			who |= S_IRWXU|S_ISUID|S_IRWXG|S_ISGID|S_IRWXO;
+			continue;
+		}
+		break;
+	}
+	if(who) {
+		clear = who;
+	} else {
+		clear = S_ISUID|S_ISGID|S_ISVTX|S_IRWXU|S_IRWXG|S_IRWXO;
+		who = ~mask;
+	}
+	while(*p) {
+		switch(*p) {
                 /* opers */
                 case '=':
                 case '+':
                 case '-':
                         op = (int)*p;
                         break;
-		/* modes */
-		case 'r':
-			m |= S_IRUSR|S_IRGRP|S_IROTH;
-			break;
-		case 'w':
-			m |= S_IWUSR|S_IWGRP|S_IWOTH;
-			break;
-		case 'x':
-			m |= S_IXUSR|S_IXGRP|S_IXOTH;
+		default:
+			eprintf("%s: invalid mode\n", str);
+			return -1;
+		}
+
+		perm = 0;
+		switch(*++p) {
+		/* copy */
+		case 'u':
+			if(mode & S_IRUSR)
+					perm |= S_IRUSR|S_IRGRP|S_IROTH;
+			if(mode & S_IWUSR)
+					perm |= S_IWUSR|S_IWGRP|S_IWOTH;
+			if(mode & S_IXUSR)
+					perm |= S_IXUSR|S_IXGRP|S_IXOTH;
+			if(mode & S_ISUID)
+					perm |= S_ISUID|S_ISGID;
+			p++;
                         break;
-		case 's':
-			m |= S_ISUID|S_ISGID;
+		case 'g':
+			if(mode & S_IRGRP)
+					perm |= S_IRUSR|S_IRGRP|S_IROTH;
+			if(mode & S_IWGRP)
+					perm |= S_IWUSR|S_IWGRP|S_IWOTH;
+			if(mode & S_IXGRP)
+					perm |= S_IXUSR|S_IXGRP|S_IXOTH;
+			if(mode & S_ISGID)
+					perm |= S_ISUID|S_ISGID;
+			p++;
                         break;
-		case 't':
-			m |= S_ISVTX;
+		case 'o':
+			if(mode & S_IROTH)
+					perm |= S_IRUSR|S_IRGRP|S_IROTH;
+			if(mode & S_IWOTH)
+					perm |= S_IWUSR|S_IWGRP|S_IWOTH;
+			if(mode & S_IXOTH)
+					perm |= S_IXUSR|S_IXGRP|S_IXOTH;
+			p++;
                         break;
                 default:
-			eprintf("%s: invalid mode\n", str);
-			return -1;
+			for(; *p; p++) {
+				switch(*p) {
+				/* modes */
+				case 'r':
+					perm |= S_IRUSR|S_IRGRP|S_IROTH;
+					break;
+				case 'w':
+					perm |= S_IWUSR|S_IWGRP|S_IWOTH;
+					break;
+				case 'x':
+					perm |= S_IXUSR|S_IXGRP|S_IXOTH;
+					break;
+				case 's':
+					perm |= S_ISUID|S_ISGID;
+					break;
+				case 't':
+					perm |= S_ISVTX;
+					break;
+				default:
+					goto apply;
+				}
+			}
                 }
+
+	apply:
                 /* apply */
                 switch(op) {
+		case '=':
+			mode &= ~clear;
+			/* fallthrough */
                 case '+':
-			mode |= (m & ((~mask) & 0777));
+			mode |= perm & who;
                         break;
                 case '-':
-			mode &= (~(m & ((~mask) & 0777)));
-			break;
-		case '=':
-			mode = (m & ((~mask) & 0777));
+			mode &= ~(perm & who);
                         break;
                 }
+		/* if we hit a comma, move on to the next clause */
+		if(*p == ',') {
+			p++;
+			goto next;
+		}
         }
-	if(gmask && op != '=')
-		mode &= ~gmask;
         return mode;
 }
Received on Mon Nov 03 2014 - 11:15:39 CET