Golom contest - 0xHarry's results

Lines of code

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/core/GolomTrader.sol#L154

Vulnerability details

Impact

The use of .transfer in https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/core/GolomTrader.sol#L154) to send ether is now considered bad practice as gas costs can change which would break the code.

Proof of Concept

See stop using soliditys transfer now, and istanbul hardfork eips increasing gas costs and more.

Recommended Mitigation Steps

Recommend using call instead, and make sure to check for reentrancy.

Lines of code

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/core/GolomTrader.sol#L177

Vulnerability details

Impact

In validateOrder, require(signaturesigner == o.signer, "invalid signature") will return true when o.signer is set to address(0) and as EVM’s ecrecover function returnsaddress(0) if a signature is invalid.

Recommended Mitigation Steps

Recommend using OpenZeppelin’s ECDSA library

Recommend adding this check to validateOrder(): require(signer != address(0), "Cannot verify signatures from 0x0");