Print this page
5548 Attempt to read ACLs from Illumos NFS client is toxic

@@ -81,51 +81,103 @@
         if (!xdr_o_mode(xdrs, &objp->a_perm))
                 return (FALSE);
         return (TRUE);
 }
 
+/*
+ * Serialize and de-serialize access control attributes
+ */
 bool_t
 xdr_secattr(XDR *xdrs, vsecattr_t *objp)
 {
-        uint_t count;
+        uint_t count = 0;
+        uint_t dfacount = 0;
 
-        if (!xdr_u_int(xdrs, &objp->vsa_mask))
+        if (!xdr_u_int(xdrs, &objp->vsa_mask)) {
                 return (FALSE);
-        if (!xdr_int(xdrs, &objp->vsa_aclcnt))
+        }
+
+        /*
+         * Refuse request if we do not understand it completely.
+         * There should be at least one valid bit set in the mask and
+         * none of the unknown bits set.
+         */
+        if ((objp->vsa_mask &
+                (VSA_ACL | VSA_ACLCNT | VSA_DFACL | VSA_DFACLCNT)) == 0) {
                 return (FALSE);
-        if (objp->vsa_aclentp != NULL)
+        }
+        if ((objp->vsa_mask &
+                ~(VSA_ACL | VSA_ACLCNT | VSA_DFACL | VSA_DFACLCNT)) != 0) {
+            return (FALSE);
+        }
+
+        if (!xdr_int(xdrs, &objp->vsa_aclcnt)) {
+                return (FALSE);
+        }
+        if (objp->vsa_aclentp != NULL) {
                 count = (uint_t)objp->vsa_aclcnt;
-        else
-                count = 0;
+        }
+
         if (!xdr_array(xdrs, (char **)&objp->vsa_aclentp, &count,
-            NFS_ACL_MAX_ENTRIES, sizeof (aclent_t), (xdrproc_t)xdr_aclent))
+            NFS_ACL_MAX_ENTRIES, sizeof (aclent_t), (xdrproc_t)xdr_aclent)) {
                 return (FALSE);
+        }
+
         if (count != 0 && count != (uint_t)objp->vsa_aclcnt) {
                 /*
                  * Assign the actual array size to vsa_aclcnt before
                  * aborting on error
                  */
                 objp->vsa_aclcnt = (int)count;
                 return (FALSE);
         }
-        if (!xdr_int(xdrs, &objp->vsa_dfaclcnt))
+
+        /*
+         * For VSA_ACL the count should be zero or there should
+         * be array attached.
+         */
+        if ((objp->vsa_mask & VSA_ACL) != 0) {
+                if ((objp->vsa_aclcnt != 0) && (objp->vsa_aclentp == NULL)) {
+                        objp->vsa_aclcnt = 0;
                 return (FALSE);
-        if (objp->vsa_dfaclentp != NULL)
-                count = (uint_t)objp->vsa_dfaclcnt;
-        else
-                count = 0;
-        if (!xdr_array(xdrs, (char **)&objp->vsa_dfaclentp, &count,
-            NFS_ACL_MAX_ENTRIES, sizeof (aclent_t), (xdrproc_t)xdr_aclent))
+                }
+        }
+
+        if (!xdr_int(xdrs, &objp->vsa_dfaclcnt)) {
                 return (FALSE);
-        if (count != 0 && count != (uint_t)objp->vsa_dfaclcnt) {
+        }
+        if (objp->vsa_dfaclentp != NULL) {
+                dfacount = (uint_t)objp->vsa_dfaclcnt;
+        }
+
+        if (!xdr_array(xdrs, (char **)&objp->vsa_dfaclentp, &dfacount,
+            NFS_ACL_MAX_ENTRIES, sizeof (aclent_t), (xdrproc_t)xdr_aclent)) {
+                return (FALSE);
+        }
+
+        if (dfacount != 0 && dfacount != (uint_t)objp->vsa_dfaclcnt) {
                 /*
                  * Assign the actual array size to vsa_dfaclcnt before
                  * aborting on error
                  */
-                objp->vsa_dfaclcnt = (int)count;
+                objp->vsa_dfaclcnt = (int)dfacount;
                 return (FALSE);
         }
+
+        /*
+         * for VSA_DFACL The count should be zero or there should
+         * be array attached
+         */
+        if ((objp->vsa_mask & VSA_DFACL) != 0) {
+                if ((objp->vsa_dfaclcnt != 0) &&
+                    (objp->vsa_dfaclentp == NULL)) {
+                        objp->vsa_dfaclcnt = 0;
+                        return (FALSE);
+                }
+        }
+
+
         return (TRUE);
 }
 
 bool_t
 xdr_GETACL2args(XDR *xdrs, GETACL2args *objp)