code-423n4 / 2024-08-wildcat-findings

3 stars 1 forks source link

`addRoleProvider` will revert if `providerAddress` is an EOA, while documentation state provider can be an EOA #74

Closed howlbot-integration[bot] closed 2 months ago

howlbot-integration[bot] commented 2 months ago

Lines of code

https://github.com/code-423n4/2024-08-wildcat/blob/main/src/access/AccessControlHooks.sol#L220-L220 https://github.com/code-423n4/2024-08-wildcat/blob/main/src/access/FixedTermLoanHooks.sol#L257-L257

Vulnerability details

Summary

addRoleProvider will revert if providerAddress is an EOA, while documentation state provider can be an EOA.

Vulnerability details

Role providers can grant credentials to lenders by calling the grantRole function.

The project documentation describe the role provider interface, and says:

Role providers can "push" credentials to the hooks contract by calling grantRole: [...] Role providers do not have to implement any of these functions - a role provider can be an EOA.

But if we try to set an EOA as a providerAddress when calling addRoleProvider, the call will revert as isPullProvider is not implemented on providerAddress (which is an EOA).

File: src/access/AccessControlHooks.sol
217:   function addRoleProvider(address providerAddress, uint32 timeToLive) external onlyBorrower {
218:     RoleProvider provider = _roleProviders[providerAddress];
219:     if (provider.isNull()) {
220:❌    bool isPullProvider = IRoleProvider(providerAddress).isPullProvider();
221:       // Role providers that are not pull providers have `pullProviderIndex` set to
222:       // `NotPullProviderIndex` (max uint24) to indicate they do not refresh credentials.
223:       provider = encodeRoleProvider(
224:         timeToLive,
225:         providerAddress,
226:         isPullProvider ? uint24(_pullProviders.length) : NotPullProviderIndex
227:       );

Also, _tryValidateCredential throw error if validateCredential return data < 32 bites, which means that if last provider for a user was an EOA, he will revert before being able to go through the pullProviders list Might want to cover this case by checking if address has code too

Impact

EOA cannot be role providers, breaking a core functionality of the protocol.

Tools Used

Manual review

Recommended Mitigation Steps

Perform a low level call to cover the case where the providerAddress has no code.

diff --git a/src/access/AccessControlHooks.sol b/src/access/AccessControlHooks.sol
index 1124e5b..0fbc0d9 100644
--- a/src/access/AccessControlHooks.sol
+++ b/src/access/AccessControlHooks.sol
@@ -217,7 +217,13 @@ contract AccessControlHooks is MarketConstraintHooks {
   function addRoleProvider(address providerAddress, uint32 timeToLive) external onlyBorrower {
     RoleProvider provider = _roleProviders[providerAddress];
     if (provider.isNull()) {
-      bool isPullProvider = IRoleProvider(providerAddress).isPullProvider();
+      bool isPullProvider;
+      (bool success, bytes memory data) = providerAddress.call(
+                               abi.encodeWithSelector(IRoleProvider.isPullProvider.selector, "")
+      );
+      if(success && data.length == 32){
+      isPullProvider = abi.decode(data, (bool));
+      }
       // Role providers that are not pull providers have `pullProviderIndex` set to
       // `NotPullProviderIndex` (max uint24) to indicate they do not refresh credentials.
       provider = encodeRoleProvider(

Assessed type

Invalid Validation

c4-judge commented 1 month ago

3docSec marked the issue as satisfactory