hats-finance / Intuition-0x538dbadc50cc87b281cd655f1edbc6ebda02a66a

The smart contracts of the Intuition protocol v1.
https://intuition.systems
Other
0 stars 1 forks source link

Withdrawals should have pausability mechanism as per docs #26

Open hats-bug-reporter[bot] opened 1 week ago

hats-bug-reporter[bot] commented 1 week ago

Github username: -- Twitter username: -- Submission hash (on-chain): 0xea09738fdf57a981c3274da26ec5f05610660bb790343ae13de21c0c2d091429 Severity: medium

Description: Description\ As per documentaton, following functionalities should have pausability mechanism:

1) Atom wallet deployment ----------Implemented in contract 2) Atom & triple creation ----------Implemented in contract 3) Atom & triple staking (deposits) ----------Implemented in contract 4) Atom & triple share redemptions (withdrawals) ----------NOT Implemented in contract

All the three functionalities i.e atom creation, deploy and deposits for atom/tripple can only be accessed when the contract is not paused. All of the above three functionalities are protected with whenNotPaused modifier which is from openzeppelin.

However, the fourth point i.e Atom & triple share redemptions (withdrawals) does not have whenNotPaused modifiers which means that they can be accessed when the contract is paused. Inshort, this is deviating from documentation.

Recommendations\ Check the requirement of whenNotPaused for redemption functions and implement as stated in documentation.

OR

If the documentation is stale or incorrect with regards to Atom & triple share redemptions (withdrawals) then remove the Atom & triple share redemptions (withdrawals) from pausability section of documentation. This can be checked here

On issue severity\

Medium severity if documentation pausability requirment is correct for redeem functions since it breaks intended design of protocol.

Low severity if documentation is incorrect and should be corrected as per contest rules such issues are low severity

mihailo-maksa commented 6 days ago

The reported issue regarding the pausability mechanism for withdrawals in EthMultiVault.sol not aligning with the documentation has been reviewed. Here is our detailed perspective:

Documentation Correction: The documentation states that redeemAtom and redeemTriple functions should be pausable. However, this is not the case in the current implementation of the contract. This discrepancy indicates that the documentation needs to be updated to accurately reflect the intended design.

Impact Assessment: Since the absence of the whenNotPaused modifier for withdrawals does not pose a security risk or lead to user harm, this issue is primarily about aligning the documentation with the code. Ensuring accurate documentation is essential for user trust and clarity.

Severity Assessment: Given that this issue does not introduce any vulnerabilities or risks to users, it is classified as low severity. The primary concern is the inconsistency between the code and the documentation.

Conclusion: The discrepancy between the code and the documentation needs to be addressed. We recommend updating the documentation to reflect the intended design where withdrawals are not pausable. This issue is more of a documentation enhancement rather than a security vulnerability.

Status: This issue is a documentation enhancement.

Suggested Fix: Update the documentation to accurately reflect that redeemAtom and redeemTriple functions are not subject to the whenNotPaused modifier. Ensure that all references to pausability are consistent with the actual implementation in the contract.

Comment for the Reporter: Thank you for highlighting this discrepancy. The documentation incorrectly states that redeemAtom and redeemTriple should be pausable, which was not intended in the code. Since this does not harm users and is more of a documentation issue, we classify it as a low severity enhancement. We can still consider a lower payout for this valid suggestion.

0xRizwan commented 14 hours ago

@mihailo-maksa This should be labelled as minor issue since documentation were tagged earlier.