code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/SafEth/derivatives/SfrxEth.sol#L60 https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/SafEth/derivatives/WstEth.sol#L56 https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/SafEth/derivatives/Reth.sol#L107

Vulnerability details

Summary

In the line 58 you can see comment says [_amount - Amount to withdraw ] but in function withdraw() first calculates specifc ETH should be withdraw corresponding to _amount parameter but in line 84 send full balance of SfrxEth.sol contract instead of specific amount . Funds will not loss because it call by admins and admins can replace it back but sending full balance .

Same Code in Reth.sol the function withdraw() first burns the mentioned _amount but in line 110 send full balance of Reth.sol . Here there chance of loss of funds because admin mentioned _amount but he gets different amount . Burned amount can recover by even admin also .

Impact

Sending full balance of deravities contract instead of mentioned amount .

Proof of Concept

https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/SafEth/derivatives/SfrxEth.sol#L84 https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/SafEth/derivatives/WstEth.sol#L63-- Same https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/SafEth/derivatives/Reth.sol#L110 -- Same

Tools Used

Manual View

Recommended Mitigation Steps

Instead of sending full balance of deravities contracts to admin can send calculated and mentioned amount
If withdraw function want to send full balance deravities contract we can remove _amount parameter .

c4-pre-sort commented 1 year ago

0xSorryNotSorry marked the issue as low quality report

c4-pre-sort commented 1 year ago

0xSorryNotSorry marked the issue as duplicate of #456

c4-judge commented 1 year ago

Picodes changed the severity to QA (Quality Assurance)