Omniscia SoMee Audit

SoMeeToken Manual Review Findings

SoMeeToken Manual Review Findings

SMT-01M: Improper Overriding

TypeSeverityLocation
Logical FaultMajorSoMeeToken.sol:L35-L42

Description:

The transferFrom function of the ERC20 contract was overridden to support the whenNotPaused modifier incorrectly so as it now performs an arbitrary _transfer between accounts and does not check the allowance.

Example:

contracts/SoMeeToken.sol
35function transferFrom(
36 address from,
37 address to,
38 uint256 value
39) public override(ERC20) whenNotPaused() returns (bool) {
40 _transfer(from, to, value);
41 return true;
42}

Recommendation:

We strongly recommend the linked function's as well as the other contract's functions override pattern to be done so by directly invoking the ERC20 implementation i.e. ERC20.transferFrom or super.transferFrom to avoid this type of issue from arising.

Alleviation:

The super implementation of transferFrom is now invoked and properly adjusts the allowance of the caller.

SMT-02M: Non-Uniform Pause Mechanism

TypeSeverityLocation
Logical FaultMinorSoMeeToken.sol:L12-L82

Description:

The pause mechanism introduced by the Manager implementation is not properly adhered to by the burn and burnFrom functions of the token's ERC20Burnable implementation.

Example:

contracts/SoMeeToken.sol
12contract SoMeeToken is ERC20, ERC20Capped, ERC20Burnable, Manager {

Recommendation:

We strongly recommend the burn and burnFrom functions of the token to also be overridden to pause when the contract is set so.

Alleviation:

The burn and burnFrom functions are now correctly overridden and enforce the pausability mechanism.

SMT-03M: Potentially Misbehaving Override

Description:

The _beforeTokenTransfer function is overridden from two contracts yet the super keyword is utilized within.

Example:

contracts/SoMeeToken.sol
75function _beforeTokenTransfer(
76 address from,
77 address to,
78 uint256 amount
79) internal virtual override(ERC20, ERC20Capped) {
80 super._beforeTokenTransfer(from, to, amount);
81}

Recommendation:

As Solidity conforms to the Python inheritence model, it is much wiser to instead directly invoke the implementation desired which in this case is ERC20Capped._beforeTokenTransfer.

Alleviation:

The ERC20Capped handler is now explicitly specified ensuring that no ambiguity may arise from the code.