Omniscia Tokemak Network Audit

Rewards Manual Review Findings

Rewards Manual Review Findings

REW-01M: Improper Cross Chain Replay Attack Protection

Description:

The current domainSeparator of the contract is initialized during the contract's constructor and thus will reflect an incorrect separator if the blockchain the contract is deployed in forks, permitting cross-chain replay attacks. A protection measure is included in claim but it is insufficient.

Example:

contracts/rewards/Rewards.sol
116function claim(
117    Recipient calldata recipient,
118    uint8 v,
119    bytes32 r,
120    bytes32 s // bytes calldata signature
121) external {
122    address signatureSigner = ecrecover(_hash(recipient), v, r, s);
123    require(signatureSigner == rewardsSigner, "Invalid Signature");
124    require(recipient.chainId == _getChainID(), "Invalid chainId");
125    require(recipient.nonce > accountNonces[recipient.wallet], "Nonce Mismatch");        
126    require(recipient.wallet == msg.sender, "Sender wallet Mismatch");
127

128    uint256 claimableAmount = recipient.amount.sub(claimedAmounts[recipient.wallet]);
129

130    require(claimableAmount > 0, "Invalid claimable amount");
131    require(tokeToken.balanceOf(address(this)) >= claimableAmount, "Insufficient Funds");
132

133    accountNonces[recipient.wallet] = recipient.nonce;
134    claimedAmounts[recipient.wallet] = claimedAmounts[recipient.wallet].add(claimableAmount);
135

136    tokeToken.safeTransfer(recipient.wallet, claimableAmount);
137

138    emit Claimed(recipient.nonce, recipient.wallet, claimableAmount);
139}

Recommendation:

We advise the protection measure to not rely on the inputs of claim and instead validate the chain ID that was initially stored during the contract's creation. This can be done by introducing a new variable that holds the said value. Additionally, the draft implementation by OpenZeppelin may also be utilized that is more comprehensive in how cross-chain signatures are handled.

Alleviation:

The Tokemak team stated that the argument based chain ID protection is a sufficient security measure. We should note that we do not believe it to be so given that it can be secured by immutable code, however, the attack vector is minimal if existent given that it requires a lot of rare conditions to materialize and thus we do not believe any action should be carried to remediate this finding.

REW-02M: Non-Standard Nonce System

TypeSeverityLocation
Logical FaultMinorRewards.sol:L125, L133

Description:

The nonce system utilized by the reward contract is non-standard in the sense that it does not sequentially increment a nonce and instead accepts any value greater than the last one which can cause issues if a very high value is specified.

Example:

contracts/rewards/Rewards.sol
116function claim(
117    Recipient calldata recipient,
118    uint8 v,
119    bytes32 r,
120    bytes32 s // bytes calldata signature
121) external {
122    address signatureSigner = ecrecover(_hash(recipient), v, r, s);
123    require(signatureSigner == rewardsSigner, "Invalid Signature");
124    require(recipient.chainId == _getChainID(), "Invalid chainId");
125    require(recipient.nonce > accountNonces[recipient.wallet], "Nonce Mismatch");        
126    require(recipient.wallet == msg.sender, "Sender wallet Mismatch");
127

128    uint256 claimableAmount = recipient.amount.sub(claimedAmounts[recipient.wallet]);
129

130    require(claimableAmount > 0, "Invalid claimable amount");
131    require(tokeToken.balanceOf(address(this)) >= claimableAmount, "Insufficient Funds");
132

133    accountNonces[recipient.wallet] = recipient.nonce;
134    claimedAmounts[recipient.wallet] = claimedAmounts[recipient.wallet].add(claimableAmount);
135

136    tokeToken.safeTransfer(recipient.wallet, claimableAmount);
137

138    emit Claimed(recipient.nonce, recipient.wallet, claimableAmount);
139}

Recommendation:

We advise the code block to instead use a sequentially increasing nonce that is designed as being equal to the recipient.nonce prior to being incremented, thus cementing a more standard and maintainable nonce system.

Alleviation:

The nonce system was dropped entirely as the system's method of signature formation disallows signatures to be replayed as the balance utilized is not stored in the contract and is instead carried within the signature payload.

REW-03M: Signature Malleability

TypeSeverityLocation
Logical FaultMinorRewards.sol:L122

Description:

The claim function utilizes a raw ecrecover invocation instead of using a well-known package, such as ECDSA, thus permitting invalid values for both the s and v values.

Example:

contracts/rewards/Rewards.sol
116function claim(
117    Recipient calldata recipient,
118    uint8 v,
119    bytes32 r,
120    bytes32 s // bytes calldata signature
121) external {
122    address signatureSigner = ecrecover(_hash(recipient), v, r, s);
123    require(signatureSigner == rewardsSigner, "Invalid Signature");
124    require(recipient.chainId == _getChainID(), "Invalid chainId");
125    require(recipient.nonce > accountNonces[recipient.wallet], "Nonce Mismatch");        
126    require(recipient.wallet == msg.sender, "Sender wallet Mismatch");
127

128    uint256 claimableAmount = recipient.amount.sub(claimedAmounts[recipient.wallet]);
129

130    require(claimableAmount > 0, "Invalid claimable amount");
131    require(tokeToken.balanceOf(address(this)) >= claimableAmount, "Insufficient Funds");
132

133    accountNonces[recipient.wallet] = recipient.nonce;
134    claimedAmounts[recipient.wallet] = claimedAmounts[recipient.wallet].add(claimableAmount);
135

136    tokeToken.safeTransfer(recipient.wallet, claimableAmount);
137

138    emit Claimed(recipient.nonce, recipient.wallet, claimableAmount);
139}

Recommendation:

We advise proper sanitization of those values by firstly ensuring s is less-than-or-equal-to secp256k1n / 2, or 0x7F..FF5D576E7357A4501DDFE92F46681B20A0, and secondly ensuring that v is equal to either 27 or 28. Values outside of those ranges will cause signatures that can be replayed. The designation of this issue is minor given that the nonce system applied protects against replays, however, such signatures should be properly prohibited.

Alleviation:

The recover function of the ECDSA OpenZeppelin library is now utilized properly enforcing the necessary checks against malleability.

Prev

Redeem.sol (RED-M)

Next

Toke.sol (TOK-M)

On this page