Use of `transfer()` instead of `call()` to send eth

[code423n4]

Lines of code

https://github.com/code-423n4/2022-10-holograph/blob/24bc4d8dfeb6e4328d2c6291d20553b1d3eff00b/contracts/HolographOperator.sol#L400 https://github.com/code-423n4/2022-10-holograph/blob/24bc4d8dfeb6e4328d2c6291d20553b1d3eff00b/contracts/HolographOperator.sol#L596 https://github.com/code-423n4/2022-10-holograph/blob/24bc4d8dfeb6e4328d2c6291d20553b1d3eff00b/contracts/HolographOperator.sol#L932 https://github.com/code-423n4/2022-10-holograph/blob/24bc4d8dfeb6e4328d2c6291d20553b1d3eff00b/contracts/enforcer/PA1D.sol#L396 https://github.com/code-423n4/2022-10-holograph/blob/24bc4d8dfeb6e4328d2c6291d20553b1d3eff00b/contracts/HolographOperator.sol#L932 https://github.com/code-423n4/2022-10-holograph/blob/24bc4d8dfeb6e4328d2c6291d20553b1d3eff00b/contracts/enforcer/PA1D.sol#L396 https://github.com/code-423n4/2022-10-holograph/blob/24bc4d8dfeb6e4328d2c6291d20553b1d3eff00b/contracts/enforcer/PA1D.sol#L416 https://github.com/code-423n4/2022-10-holograph/blob/24bc4d8dfeb6e4328d2c6291d20553b1d3eff00b/contracts/enforcer/PA1D.sol#L439

Vulnerability details

Use of transfer() instead of call() to send eth

Impact

Use of transfer() might render ETH impossible to withdraw because after istanbul hardfork, there is increases in the gas cost of the SLOAD operation and therefore breaks some existing smart contracts.Those contracts will break because their fallback functions used to consume less than 2300 gas, and they’ll now consume more, since 2300 the amount of gas a contract’s fallback function receives if it’s called via Solidity’s transfer() or send() methods.

Any smart contract that uses transfer() or send() is taking a hard dependency on gas costs by forwarding a fixed amount of gas: 2300.

References

https://consensys.net/diligence/blog/2019/09/stop-using-soliditys-transfer-now/ https://blog.openzeppelin.com/opyn-gamma-protocol-audit/

Proof of Concept

https://github.com/code-423n4/2022-10-holograph/blob/24bc4d8dfeb6e4328d2c6291d20553b1d3eff00b/contracts/HolographOperator.sol#L400 _utilityToken().transfer(job.operator, leftovers);

https://github.com/code-423n4/2022-10-holograph/blob/24bc4d8dfeb6e4328d2c6291d20553b1d3eff00b/contracts/HolographOperator.sol#L596 payable(hToken).transfer(hlgFee);

https://github.com/code-423n4/2022-10-holograph/blob/24bc4d8dfeb6e4328d2c6291d20553b1d3eff00b/contracts/HolographOperator.sol#L932 require(_utilityToken().transfer(recipient, amount), "HOLOGRAPH: token transfer failed");

https://github.com/code-423n4/2022-10-holograph/blob/24bc4d8dfeb6e4328d2c6291d20553b1d3eff00b/contracts/enforcer/PA1D.sol#L396 addresses[i].transfer(sending);

https://github.com/code-423n4/2022-10-holograph/blob/24bc4d8dfeb6e4328d2c6291d20553b1d3eff00b/contracts/enforcer/PA1D.sol#L416 require(erc20.transfer(addresses[i], sending), "PA1D: Couldn't transfer token");

https://github.com/code-423n4/2022-10-holograph/blob/24bc4d8dfeb6e4328d2c6291d20553b1d3eff00b/contracts/enforcer/PA1D.sol#L439 require(erc20.transfer(addresses[i], sending), "PA1D: Couldn't transfer token");

Recommended Mitigation Steps

Use call() to send eth

[gzeoneth]

Duplicate of #33

[ACC01ADE]

The statement to use call instead of transfer is an opinion and not a standard. We extensively and purposefully use transfer in our protocol. Sponsor acknowledges this issue, but believes this is a low severity at best.

[gzeoneth]

Consider with #492