-
Notifications
You must be signed in to change notification settings - Fork 2
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
- Loading branch information
1 parent
d772c55
commit f08ea0b
Showing
1 changed file
with
37 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,37 @@ | ||
1. Inefficient Credential Validation: Potential Double-Check of Same Provider in AccessControlHooks Contract | ||
|
||
https://github.com/code-423n4/2024-08-wildcat/blob/fe746cc0fbedc4447a981a50e6ba4c95f98b9fe1/src/access/AccessControlHooks.sol#L670-L680 | ||
|
||
```solidity | ||
// Handle the calldata suffix, if any | ||
if (_handleHooksData(status, accountAddress, hooksData)) { | ||
return (true, true); | ||
} | ||
uint256 pullProviderIndexToSkip = type(uint256).max; | ||
// If lender has an expired credential from a pull provider, attempt to refresh it | ||
if (!lastProvider.isNull() && status.canRefresh) { | ||
if (_tryGetCredential(status, lastProvider, accountAddress)) { | ||
return (true, true); | ||
} | ||
// If refresh fails, provider should be skipped in the query loop | ||
pullProviderIndexToSkip = lastProvider.pullProviderIndex(); | ||
} | ||
``` | ||
|
||
If the hooksData has a length of 20 bytes (which represents just a provider address), _handleHooksData will attempt to get a credential from that provider using _tryGetCredential. | ||
If that fails, the code then checks if there's a lastProvider and attempts to refresh the credential using the same _tryGetCredential method. | ||
If the provider in hooksData (with length 20) is the same as lastProvider, this would result in trying to get a credential from the same provider twice. | ||
|
||
This redundancy could lead to unnecessary gas consumption and potentially confusing behavior. | ||
|
||
|
||
|
||
2. In HooksFactory, once a hooks template is disabled, there's no way to re-enable it. | ||
`disableHooksTemplate` function sets enabled to false, but there's no corresponding function to set it back to true. https://github.com/code-423n4/2024-08-wildcat/blob/fe746cc0fbedc4447a981a50e6ba4c95f98b9fe1/src/HooksFactory.sol#L201 | ||
|
||
|
||
|
||
3.The HooksFactory contract allows the creation of new markets using hooks instances derived from disabled templates. While the contract prevents the deployment of new hooks instances from disabled templates, it fails to check the enabled status of a template when deploying a market with an existing hooks instance. This oversight potentially allows the continued use of hooks logic that has been identified as problematic or insecure, bypassing the intended security measure of disabling templates. https://github.com/code-423n4/2024-08-wildcat/blob/fe746cc0fbedc4447a981a50e6ba4c95f98b9fe1/src/HooksFactory.sol#L502 |