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
---
 util/mode.c | 129 ++++++++++++++++++++++++++++++++++++++++++++----------------
 1 file changed, 95 insertions(+), 34 deletions(-)

diff --git a/util/mode.c b/util/mode.c
index 4506023..c022e8d 100644
--- a/util/mode.c
+++ b/util/mode.c
@@ -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') {
@@ -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;
 }
-- 
2.1.3.1.g339ec9c


Reply via email to