Liquid Staking

Stakelink
DeFiHardhatOracle
50,000 USDC
View results
Submission Details
Severity: medium
Invalid

contracts/core/tokens/base/ERC677Upgradeable.sol

The provided Solidity code implements an upgradeable ERC677 token using OpenZeppelin's contracts. While it generally follows best practices, there are a few potential vulnerabilities and issues to consider:

1. Lack of Reentrancy Guard

  • Issue: The transferAndCall and transferAndCallFrom functions invoke an external contract method (onTokenTransfer) after transferring tokens. This could expose the contract to reentrancy attacks.

  • Mitigation: Consider using a reentrancy guard (e.g., from OpenZeppelin's ReentrancyGuard) to protect against reentrancy attacks. You can implement a mutex that prevents reentrant calls during execution of these functions.

2. Improper Handling of ERC20 Transfer Failures

  • Issue: The transferAndCall method does not check if the transfer was successful before proceeding to call the external contract. If the transfer fails (for example, due to insufficient balance), the fallback function could still be executed, leading to unexpected behavior.

  • Mitigation: Use require() to ensure that the transfer succeeded before calling the fallback. For example:

    require(super.transfer(_to, _value), "Transfer failed");

3. Missing Access Control

  • Issue: The __ERC677_init function does not implement any access control. As it is intended to initialize the contract, anyone could call it, potentially leading to improper initialization or misuse.

  • Mitigation: Consider adding an onlyOwner modifier (or a similar access control mechanism) to restrict who can initialize the contract.

4. Gas Limit Issues

  • Issue: If the contractFallback function is called and the receiving contract does not handle the call properly (e.g., it consumes too much gas or reverts), the entire transaction will fail.

  • Mitigation: There is no straightforward solution here, but you might want to document this behavior and inform users to ensure that any contract they interact with implements the onTokenTransfer method correctly.

5. Overflow/Underflow Protection

  • Issue: While Solidity 0.8.x has built-in overflow and underflow protection, make sure to properly handle situations where _totalSupply is zero in the constructor or check for any potential arithmetic issues in other functions.

  • Mitigation: Ensure any arithmetic operations in the contract handle edge cases.

6. No Event Emission for Transfers

  • Issue: The contract does not emit any events for the transferAndCall or transferAndCallFrom functions. While standard ERC20 transfers emit events, the additional functionality should also log when tokens are transferred alongside a call.

  • Mitigation: Emit relevant events to keep track of these actions:

    emit Transfer(msg.sender, _to, _value);

Example of Improved Code

Here's a revised version of the transferAndCall function considering some of the above points:

function transferAndCall(
address _to,
uint256 _value,
bytes memory _data
) public returns (bool) {
require(super.transfer(_to, _value), "Transfer failed");
if (isContract(_to)) {
contractFallback(msg.sender, _to, _value, _data);
}
emit Transfer(msg.sender, _to, _value); // Emit event
return true;
}

Conclusion

Review these points carefully and apply the necessary mitigations to enhance the security and robustness of your smart contract. Always conduct thorough testing, including unit tests and fuzz testing, to ensure your contract behaves as expected under various conditions.

Updates

Lead Judging Commences

inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Lack of quality

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.