Good Entry - hpsb's results

Lines of code

https://github.com/code-423n4/2023-08-goodentry/blob/71c0c0eca8af957202ccdbf5ce2f2a514ffe2e24/contracts/helper/V3Proxy.sol#L156 https://github.com/code-423n4/2023-08-goodentry/blob/71c0c0eca8af957202ccdbf5ce2f2a514ffe2e24/contracts/helper/V3Proxy.sol#L174 https://github.com/code-423n4/2023-08-goodentry/blob/71c0c0eca8af957202ccdbf5ce2f2a514ffe2e24/contracts/helper/V3Proxy.sol#L192

Vulnerability details

Impact

The functions do not check the return value of low-level calls. This can lock Ether in the contract if the call fails or may compromise the contract if the ownership is being changed. The following calls were detected without return value validations - call

Proof of Concept

Check both the links: https://swcregistry.io/docs/SWC-104/ https://coinsbench.com/unchecked-return-value-in-smart-contracts-providing-an-attack-surface-dab2eed64251#:~:text=When%20the%20return%20value%20of,be%20executed%20causing%20unexpected%20behavior.

Tools Used

Manual Review

Recommended Mitigation Steps

Ensure return value is checked using conditional statements for low-level calls. We should also ensure that we log failed calls using events.

Assessed type

call/delegatecall

Lines of code

https://github.com/code-423n4/2023-08-goodentry/blob/71c0c0eca8af957202ccdbf5ce2f2a514ffe2e24/contracts/TokenisableRange.sol#L228 https://github.com/code-423n4/2023-08-goodentry/blob/71c0c0eca8af957202ccdbf5ce2f2a514ffe2e24/contracts/TokenisableRange.sol#L229

Vulnerability details

Impact

Some tokens do not revert the transaction when the transfer or transferFrom fails and returns False. Hence we must check the return value after calling the transfer or transferFrom function.

Proof of Concept

Check the last answer here: https://ethereum.stackexchange.com/questions/136039/what-is-best-practice-for-transferfrom-out-of-these-two-ways In short: So using token.transferFrom(msg.sender, addr, amount); is NOT enough. The risk is that you're assuming the transfer was successful when actually it wasn't. Example of tokens that don't revert: ZRX, HT, WOO. The safest method is using this:

(bool success, bytes memory data) = address(token).call(abi.encodeWithSelector(token.transferFrom.selector, from, to, value)); require(success && (data.length == 0 || abi.decode(data, (bool))), 'token transfer from sender failed'); This is the one used by Openzeppelin's SafeERC20 library.

Tools Used

Manual Report

Recommended Mitigation Steps

Use OpenZeppelin SafeERC20's safetransfer and safetransferFrom functions.

Assessed type

Token-Transfer