REENTRANCY-Avoid transfer() as reentrancy mitigations. Transfer do not protect from reentrancies in case of gas price changes.

[code423n4]

Lines of code

https://github.com/code-423n4/2022-06-nibbl/blob/8c3dbd6adf350f35c58b31723d42117765644110/contracts/Basket.sol#L78-L82

Vulnerability details

Impact --Check: reentrancy-unlimited-gas --Severity: Informational --Confidence: Medium Transfer() and send() have been recommended as a security best-practice to prevent reentrancy attacks because they only forward 2300 gas. That being said, gas repricing of opcodes may break deployed contracts.

Proof of Concept

---ChainSecurity - Ethereum Istanbul Hardfork: The Security Perspective ---Steve Marx - Stop Using Solidity’s transfer() Now ---EIP 1884 ---Consensys - https://consensys.net/diligence/blog/2019/09/stop-using-soliditys-transfer-now/

Tools Used --Slither

Recommended Mitigation Steps Use call() instead, without hardcoded gas limits along with checks-effects-interactions pattern or reentrancy guards for reentrancy protection.

Ex: contract Vulnerable { function withdraw(uint256 amount) external { // This forwards 2300 gas, which may not be enough if the recipient // is a contract and gas costs change. msg.sender.transfer(amount); } }

contract Fixed { function withdraw(uint256 amount) external { // This forwards all available gas. Be sure to check the return value! (bool success, ) = msg.sender.call.value(amount)(""); require(success, "Transfer failed."); } }

[mundhrakeshav]

18

[GalloDaSballo]

Finding looks like it was copy pasted from an automated tool

[HardlyDifficult]

The potential reentrant call happens after state changes so there is no relevant risk here.