Borrower can REVOKE A USER'S CREDENTIAL by Blocking the user instead of just preventing deposits, while the unblocking function if called will not return back the User's Revoked Role. #81
Only Registered providers should be able to approve and revoke users credentials but these act can be executed by the borrow while trying to prevent a user from depositing.
This issue is magnified because, This action of revoking the Users role when blocking is not returned when the user is unblocked hence a borrower can block then unblock a user but their roles have been revoked in this process and will not be available because the role are not not set again.
The former implemetation by wildcat is straight forward has the lender can add and remove lenders roles easily.
/**
* @dev Grant authorization for a set of lenders.
*
* Note: Only updates the internal set of approved lenders.
* Must call `updateLenderAuthorization` to apply changes
* to existing market accounts
*/
function authorizeLenders(address[] memory lenders) external onlyBorrower {
for (uint256 i = 0; i < lenders.length; i++) {
address lender = lenders[i];
if (_authorizedLenders.add(lender)) {
emit LenderAuthorized(lender);
}
}
}
/**
* @dev Revoke authorization for a set of lenders.
*
* Note: Only updates the internal set of approved lenders.
* Must call `updateLenderAuthorization` to apply changes
* to existing market accounts
*/
function deauthorizeLenders(address[] memory lenders) external onlyBorrower {
for (uint256 i = 0; i < lenders.length; i++) {
address lender = lenders[i];
if (_authorizedLenders.remove(lender)) {
emit LenderDeauthorized(lender);
}
}
}
But presently these roles have been delegated to Providers even if borrowers is also a provider, unblocking a user should return their credential if blocking a user unsets their credentials.
function revokeRole(address account) external {
LenderStatus memory status = _lenderStatus[account];
if (status.lastProvider != msg.sender) {
revert ProviderCanNotRevokeCredential();
}
status.unsetCredential();
_lenderStatus[account] = status;
emit AccountAccessRevoked(account);
}
The vulnerability lies in how the system handles the blocking and unblocking of accounts:
Blocking a User:
The borrower's ability to block a user also revokes their credentials and their access roles:
function blockFromDeposits(address account) external onlyBorrower {
LenderStatus memory status = _lenderStatus[account];
@audit >> revokes role too>> if (status.hasCredential()) {
status.unsetCredential();
emit AccountAccessRevoked(account);
}
status.isBlockedFromDeposits = true;
_lenderStatus[account] = status;
emit AccountBlockedFromDeposits(account);
}
Borrower in another extreme turn around can block users and also remove the RoleProviders that they are registered with.
/**
* @dev Removes a role provider from the `_roleProviders` mapping and, if it is a
* pull provider, from the `_pullProviders` array.
*/
function removeRoleProvider(address providerAddress) external onlyBorrower {
RoleProvider provider = _roleProviders[providerAddress];
if (provider.isNull()) revert ProviderNotFound();
// Remove the provider from `_roleProviders`
_roleProviders[providerAddress] = EmptyRoleProvider;
emit RoleProviderRemoved(providerAddress, provider.pullProviderIndex());
// If the provider is a pull provider, remove it from `_pullProviders`
if (provider.isPullProvider()) {
_removePullProvider(provider.pullProviderIndex());
}
}
Unblocking a User:
When the user is unblocked, their access role is not restored, leading to the user remaining in a blocked state, unable to be validated or perform transactions:
function unblockFromDeposits(address account) external onlyBorrower {
LenderStatus memory status = _lenderStatus[account];
@audit >> doesn't grant role back>>
status.isBlockedFromDeposits = false;
_lenderStatus[account] = status;
emit AccountUnblockedFromDeposits(account);
}
Credential Validation:
The account is checked for expired credentials, but since the refresh flag is set to false and no valid provider is found, the user remains in a blocked state, causing DoS:
if (!lastProvider.isNull() && status.canRefresh) {
if (_tryGetCredential(status, lastProvider, accountAddress)) {
return (true, true);
}
}
Tools Used
Manual code analysis and review.
Recommended Mitigation Steps
The borrower can possess multiple roles but also if a user Blocks a user and Revokes his role, Unblocking the same user should Grant the user their role back
Separate the logic for revoking roles and blocking deposits for borrowers. This will prevent the system from revoking credentials during the blocking process, ensuring that the account's role remains intact.
Proposed solution:
function revokeRolebyborrower(address account) external onlyBorrower {
LenderStatus memory status = _lenderStatus[account];
status.unsetCredential();
_lenderStatus[account] = status;
emit AccountAccessRevoked(account);
}
function blockFromDeposits(address account) external onlyBorrower {
LenderStatus memory status = _lenderStatus[account];
status.isBlockedFromDeposits = true;
_lenderStatus[account] = status;
emit AccountBlockedFromDeposits(account);
}
Lines of code
https://github.com/code-423n4/2024-08-wildcat/blob/fe746cc0fbedc4447a981a50e6ba4c95f98b9fe1/src/access/FixedTermLoanHooks.sol#L406 https://github.com/code-423n4/2024-08-wildcat/blob/fe746cc0fbedc4447a981a50e6ba4c95f98b9fe1/src/access/FixedTermLoanHooks.sol#L482-L491 https://github.com/code-423n4/2024-08-wildcat/blob/fe746cc0fbedc4447a981a50e6ba4c95f98b9fe1/src/access/FixedTermLoanHooks.sol#L493-L498 https://github.com/code-423n4/2024-08-wildcat/blob/fe746cc0fbedc4447a981a50e6ba4c95f98b9fe1/src/types/LenderStatus.sol#L55-L70
Vulnerability details
Proof of Concept
Only Registered providers should be able to approve and revoke users credentials but these act can be executed by the borrow while trying to prevent a user from depositing. This issue is magnified because, This action of revoking the Users role when blocking is not returned when the user is unblocked hence a borrower can block then unblock a user but their roles have been revoked in this process and will not be available because the role are not not set again.
The former implemetation by wildcat is straight forward has the lender can add and remove lenders roles easily.
But presently these roles have been delegated to Providers even if borrowers is also a provider, unblocking a user should return their credential if blocking a user unsets their credentials.
The vulnerability lies in how the system handles the blocking and unblocking of accounts:
Borrower in another extreme turn around can block users and also remove the RoleProviders that they are registered with.
Credential Validation: The account is checked for expired credentials, but since the refresh flag is set to false and no valid provider is found, the user remains in a blocked state, causing DoS:
Tools Used
Recommended Mitigation Steps
The borrower can possess multiple roles but also if a user Blocks a user and Revokes his role, Unblocking the same user should Grant the user their role back
Separate the logic for revoking roles and blocking deposits for borrowers. This will prevent the system from revoking credentials during the blocking process, ensuring that the account's role remains intact.
Proposed solution:
Assessed type
Error