code-423n4 / 2023-01-ondo-findings

0 stars 0 forks source link

Deny of service in CCash.sol with "transfer" which can be unusable for smart contract calls #287

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-01-ondo/blob/f3426e5b6b4561e09460b2e6471eb694efdd6c70/contracts/lending/tokens/cCash/CCash.sol#L236-L260

Vulnerability details

Impact The CCash.doTransferOut method is susceptible to denial of service.

Proof of Concept The logic of the doTransferOut method in CCash is as follows:

https://github.com/code-423n4/2023-01-ondo/blob/f3426e5b6b4561e09460b2e6471eb694efdd6c70/contracts/lending/tokens/cCash/CCash.sol#L236-L260

The whole user withdraw is being handled with a token.transfer() call. This is unsafe as transfer has hard coded gas budget and can fail when the user is a smart contract Whenever the user either fails to implement the payable fallback function or cumulative gas cost of the function sequence invoked on a native token transfer exceeds 2300 gas consumption limit the native tokens sent end up undelivered and the corresponding user funds return functionality will fail each time. The issues with transfer() are outlined here: https://consensys.net/diligence/blog/2019/09/stop-using-soliditys-transfer-now/

01: https://github.com/code-423n4/2023-01-ondo/blob/f3426e5b6b4561e09460b2e6471eb694efdd6c70/contracts/lending/tokens/cCash/CTokenCash.sol#L650 02: https://github.com/code-423n4/2023-01-ondo/blob/f3426e5b6b4561e09460b2e6471eb694efdd6c70/contracts/lending/tokens/cCash/CTokenCash.sol#L730 03: https://github.com/code-423n4/2023-01-ondo/blob/f3426e5b6b4561e09460b2e6471eb694efdd6c70/contracts/lending/tokens/cCash/CTokenCash.sol#L1287 04: https://github.com/code-423n4/2023-01-ondo/blob/f3426e5b6b4561e09460b2e6471eb694efdd6c70/contracts/lending/tokens/cToken/CTokenModified.sol#L650 05: https://github.com/code-423n4/2023-01-ondo/blob/f3426e5b6b4561e09460b2e6471eb694efdd6c70/contracts/lending/tokens/cToken/CTokenModified.sol#L730 06: https://github.com/code-423n4/2023-01-ondo/blob/f3426e5b6b4561e09460b2e6471eb694efdd6c70/contracts/lending/tokens/cToken/CTokenModified.sol#L1290

Tools Used: Manual Review

Recommended Mitigation Steps

Using low-level call.value(amount) with the corresponding result check or using the OpenZeppelin Address.sendValue is advised: https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/utils/Address.sol#L60

c4-judge commented 1 year ago

trust1995 marked the issue as unsatisfactory: Insufficient quality

tom2o17 commented 1 year ago

Forked code: out of scope

cc: @ypatil12 @cameronclifton