code-423n4 / 2021-08-notional-findings

3 stars 0 forks source link

TokenHandler.sol, L174 - .transfer is bad practice #15

Open code423n4 opened 3 years ago

code423n4 commented 3 years ago

Handle

tensors

Vulnerability details

Impact

The use of .transfer to send ether is now considered bad practice as gas costs can change which would break the code. See: https://consensys.net/diligence/blog/2019/09/stop-using-soliditys-transfer-now/ https://chainsecurity.com/istanbul-hardfork-eips-increasing-gas-costs-and-more/

Proof of Concept

TokenHandler.sol, L174 https://github.com/code-423n4/2021-08-notional/blob/4b51b0de2b448e4d36809781c097c7bc373312e9/contracts/internal/balances/TokenHandler.sol#L174

Recommended Mitigation Steps

Use call instead, and make sure to check for reentrancy.

jeffywu commented 3 years ago

Duplicate #38