Beedle - Oracle free perpetual lending

Summary

call() should be used instead of transfer() as with EIP-1884, gas cost for SLOAD operation changed which broke many existing contracts

Vulnerability Details

The transfer() and send() functions forward a fixed amount of 2300 gas. Historically, it has often been recommended to use these functions for value transfers to guard against reentrancy attacks. However, the gas cost of EVM instructions may change significantly during hard forks which may break already deployed contract systems that make fixed assumptions about gas costs. For example. EIP 1884 broke several existing smart contracts due to a cost increase of the SLOAD instruction.

Instances:

1. 2023-07-beedle/blob/main/src/Lender.sol: setPool(): Line 152 , Line 159

2. 2023-07-beedle/blob/main/src/Lender.sol: removeFromPool(): Line 203

3. 2023-07-beedle/blob/main/src/Lender.sol: borrow(): Line 267, Line 269

4. 2023-07-beedle/blob/main/src/Lender.sol: repay(): Line 329

5. 2023-07-beedle/blob/main/src/Lender.sol: giveLoan(): Line 403

6. 2023-07-beedle/blob/main/src/Lender.sol: buyLoan(): Line 505

7. 2023-07-beedle/blob/main/src/Lender.sol: seizeLoan(): Line 563, Line 564

8. 2023-07-beedle/blob/main/src/Lender.sol: refinance(): Line 651, Line 653, Line 656, Line 670

Impact

The use of the deprecated transfer() function for an address will inevitably make the transaction fail when:

1. The claimer smart contract does not implement a payable function.

2. The claimer smart contract does implement a payable fallback which uses more than 2300 gas unit.

3. The claimer smart contract implements a payable fallback function that needs less than 2300 gas units but is called through proxy, raising the call's gas usage above 2300.

4. Additionally, using higher than 2300 gas might be mandatory for some multisig wallets.

Tools Used

VSCodium, Manual audit

Recommendations

Use the low-level function call() instead of transfer(), but be sure to respect the CEI pattern and/or add re-entrancy guards (popular choice is the one from OpenZeppelin), as several hacks have already happened in the past due to this recommendation not being fully understood.

More info on here.