Merge pull request #147 from Bananapus/33audit/m01

33Audits [M-01 / M-03]

package.json

@@ -18,7 +18,7 @@
     "artifacts": "source ./.env && npx sphinx artifacts --org-id 'cltepuu9u0003j58rjtbd0hvu' --project-name 'nana-core'"
   },
   "dependencies": {
-    "@bananapus/permission-ids": "^0.0.5",
+    "@bananapus/permission-ids": "^0.0.7",
     "@chainlink/contracts": "^1.1.0",
     "@openzeppelin/contracts": "^5.0.2",
     "@prb/math": "^4.0.2",

src/JBPermissions.sol

@@ -15,6 +15,13 @@ contract JBPermissions is IJBPermissions {
     error PERMISSION_ID_OUT_OF_BOUNDS();
     error UNAUTHORIZED();
 
+    //*********************************************************************//
+    // ------------------------- public constants ------------------------ //
+    //*********************************************************************//
+
+    /// @notice The project ID considered a wildcard, meaning it will grant permissions to all projects.
+    uint256 public constant override WILDCARD_PROJECT_ID = 0;
+
     //*********************************************************************//
     // --------------------- public stored properties -------------------- //
     //*********************************************************************//
@@ -41,53 +48,127 @@ contract JBPermissions is IJBPermissions {
     /// @param account The account being operated on behalf of.
     /// @param projectId The project ID that the operator has permission to operate under. 0 represents all projects.
     /// @param permissionId The permission ID to check for.
+    /// @param includeRoot A flag indicating if the return value should default to true if the operator has the ROOT
+    /// permission.
+    /// @param includeWildcardProjectId A flag indicating if the return value should return true if the operator has the
+    /// specified permission on the wildcard project ID.
+    /// true.
     /// @return A flag indicating whether the operator has the specified permission.
     function hasPermission(
         address operator,
         address account,
         uint256 projectId,
-        uint256 permissionId
+        uint256 permissionId,
+        bool includeRoot,
+        bool includeWildcardProjectId
     )
         public
         view
         override
         returns (bool)
     {
+        // Indexes above 255 don't exist
         if (permissionId > 255) revert PERMISSION_ID_OUT_OF_BOUNDS();
 
-        return (((permissionsOf[operator][account][projectId] >> permissionId) & 1) == 1);
+        // If the ROOT permission is set and should be included, return true.
+        if (
+            includeRoot
+                && (
+                    _includesPermission({
+                        permissions: permissionsOf[operator][account][projectId],
+                        permissionId: JBPermissionIds.ROOT
+                    })
+                        || (
+                            includeWildcardProjectId
+                                && _includesPermission({
+                                    permissions: permissionsOf[operator][account][WILDCARD_PROJECT_ID],
+                                    permissionId: JBPermissionIds.ROOT
+                                })
+                        )
+                )
+        ) {
+            return true;
+        }
+
+        // Otherwise return the t/f flag of the specified id.
+        return _includesPermission({
+            permissions: permissionsOf[operator][account][projectId],
+            permissionId: permissionId
+        })
+            || (
+                includeWildcardProjectId
+                    && _includesPermission({
+                        permissions: permissionsOf[operator][account][WILDCARD_PROJECT_ID],
+                        permissionId: permissionId
+                    })
+            );
     }
 
     /// @notice Check if an operator has all of the specified permissions for a specific address and project ID.
     /// @param operator The operator to check.
     /// @param account The account being operated on behalf of.
     /// @param projectId The project ID that the operator has permission to operate under. 0 represents all projects.
     /// @param permissionIds An array of permission IDs to check for.
+    /// @param includeRoot A flag indicating if the return value should default to true if the operator has the ROOT
+    /// permission.
+    /// @param includeWildcardProjectId A flag indicating if the return value should return true if the operator has the
+    /// specified permission on the wildcard project ID.
     /// @return A flag indicating whether the operator has all specified permissions.
     function hasPermissions(
         address operator,
         address account,
         uint256 projectId,
-        uint256[] calldata permissionIds
+        uint256[] calldata permissionIds,
+        bool includeRoot,
+        bool includeWildcardProjectId
     )
         external
         view
         override
         returns (bool)
     {
+        // If the ROOT permission is set and should be included, return true.
+        if (
+            includeRoot
+                && (
+                    _includesPermission({
+                        permissions: permissionsOf[operator][account][projectId],
+                        permissionId: JBPermissionIds.ROOT
+                    })
+                        || (
+                            includeWildcardProjectId
+                                && _includesPermission({
+                                    permissions: permissionsOf[operator][account][WILDCARD_PROJECT_ID],
+                                    permissionId: JBPermissionIds.ROOT
+                                })
+                        )
+                )
+        ) {
+            return true;
+        }
+
         // Keep a reference to the permission being iterated on.
         uint256 permissionId;
 
         // Keep a reference to the permission item being checked.
         uint256 operatorAccountProjectPermissions = permissionsOf[operator][account][projectId];
 
+        // Keep a reference to the wildcard project permissions.
+        uint256 operatorAccountWildcardProjectPermissions =
+            includeWildcardProjectId ? permissionsOf[operator][account][WILDCARD_PROJECT_ID] : 0;
+
         for (uint256 i; i < permissionIds.length; i++) {
             // Set the permission being iterated on.
             permissionId = permissionIds[i];
 
+            // Indexes above 255 don't exist
             if (permissionId > 255) revert PERMISSION_ID_OUT_OF_BOUNDS();
 
-            if (((operatorAccountProjectPermissions >> permissionId) & 1) == 0) {
+            // Check each permissionId
+            if (
+                !_includesPermission({permissions: operatorAccountProjectPermissions, permissionId: permissionId})
+                    && !_includesPermission({permissions: operatorAccountWildcardProjectPermissions, permissionId: permissionId})
+            ) {
                 return false;
             }
         }
@@ -103,16 +184,26 @@ contract JBPermissions is IJBPermissions {
     /// @param account The account setting its operators' permissions.
     /// @param permissionsData The data which specifies the permissions the operator is being given.
     function setPermissionsFor(address account, JBPermissionsData calldata permissionsData) external override {
-        // Enforce permissions.
+        // Pack the permission IDs into a uint256.
+        uint256 packed = _packedPermissions(permissionsData.permissionIds);
+
+        // Enforce permissions. ROOT operators are allowed to set permissions so long as they are not setting another
+        // ROOT permission.
         if (
             msg.sender != account
-                && !hasPermission(msg.sender, account, permissionsData.projectId, JBPermissionIds.ROOT)
-                && !hasPermission(msg.sender, account, 0, JBPermissionIds.ROOT)
+                && (
+                    _includesPermission({permissions: packed, permissionId: JBPermissionIds.ROOT})
+                        || !hasPermission({
+                            operator: msg.sender,
+                            account: account,
+                            projectId: permissionsData.projectId,
+                            permissionId: JBPermissionIds.ROOT,
+                            includeRoot: true,
+                            includeWildcardProjectId: true
+                        })
+                )
         ) revert UNAUTHORIZED();
 
-        // Pack the permission IDs into a uint256.
-        uint256 packed = _packedPermissions(permissionsData.permissionIds);
-
         // Store the new value.
         permissionsOf[permissionsData.operator][account][permissionsData.projectId] = packed;
 
@@ -147,4 +238,12 @@ contract JBPermissions is IJBPermissions {
             packed |= 1 << permissionId;
         }
     }
+
+    /// @notice Checks if a permission is included in a packed permissions data.
+    /// @param permissions The packed permissions to check.
+    /// @param permissionId The ID of the permission to check for.
+    /// @return A flag indicating whether the permission is included.
+    function _includesPermission(uint256 permissions, uint256 permissionId) internal pure returns (bool) {
+        return ((permissions >> permissionId) & 1) == 1;
+    }
 }

src/JBSplits.sol

@@ -24,7 +24,7 @@ contract JBSplits is JBControlled, IJBSplits {
     // ------------------------- public constants ------------------------ //
     //*********************************************************************//
 
-    /// @notice the ID of the ruleset that will be checked if nothing was found in the provided rulesetId.
+    /// @notice The ID of the ruleset that will be checked if nothing was found in the provided rulesetId.
     uint256 public constant override FALLBACK_RULESET_ID = 0;
 
     //*********************************************************************//

src/abstract/JBPermissioned.sol

@@ -41,8 +41,15 @@ abstract contract JBPermissioned is Context, IJBPermissioned {
     function _requirePermissionFrom(address account, uint256 projectId, uint256 permissionId) internal view {
         address sender = _msgSender();
         if (
-            sender != account && !PERMISSIONS.hasPermission(sender, account, projectId, permissionId)
-                && !PERMISSIONS.hasPermission(sender, account, 0, permissionId)
+            sender != account
+                && !PERMISSIONS.hasPermission({
+                    operator: sender,
+                    account: account,
+                    projectId: projectId,
+                    permissionId: permissionId,
+                    includeRoot: true,
+                    includeWildcardProjectId: true
+                })
         ) revert UNAUTHORIZED();
     }
 

src/interfaces/IJBPermissions.sol

@@ -13,13 +13,17 @@ interface IJBPermissions {
         address caller
     );
 
+    function WILDCARD_PROJECT_ID() external view returns (uint256);
+
     function permissionsOf(address operator, address account, uint256 projectId) external view returns (uint256);
 
     function hasPermission(
         address operator,
         address account,
         uint256 projectId,
-        uint256 permissionId
+        uint256 permissionId,
+        bool includeRoot,
+        bool includeWildcardProjectId
     )
         external
         view
@@ -29,7 +33,9 @@ interface IJBPermissions {
         address operator,
         address account,
         uint256 projectId,
-        uint256[] calldata permissionIds
+        uint256[] calldata permissionIds,
+        bool includeRoot,
+        bool includeWildcardProjectId
     )
         external
         view