Cudos contest - hake's results

LOW

ecrecover allows signature malleability

Gravity.sol#L175-L186

Auditor shw has previously stated the exact same issue in past audit.

I will simply reuse his report as I don't believe I could offer a clearer explanation.

gravitybridge-findings/issues/61

Impact

"The verifySig function of Gravity calls the Solidity ecrecover function directly to verify the given signatures. However, the ecrecover EVM opcode allows malleable (non-unique) signatures and thus is susceptible to replay attacks.

Although a replay attack seems not possible here since the nonce is increased each time, ensuring the signatures are not malleable is considered a best practice (and so is checking _signer != address(0), where address(0) means an invalid signature)."

Proof of Concept

Gravity.sol#L185

SWC-117: Signature Malleability SWC-121: Missing Protection against Signature Replay Attacks

Recommended Mitigation Steps

Use the recover function from OpenZeppelin's ECDSA library for signature verification.

GAS

for loop optimization

Gravity.sol#L128

for (uint256 i = 0; i < _users.length; i++) {
    require(
                _users[i] != address(0),
                "User is the zero address"
            );
    whitelisted[_users[i]] = _isWhitelisted;

Gas could be saved by:

• Not initializing variable to default value of zero
• Caching array length
• Using a prefix (++i) instead of a postfix (i++)
• Unchecking increment count

Example:

uint size = _users.length;

for (uint256 i = 0; i < size;) {
    require(
                _users[i] != address(0),
                "User is the zero address"
            );
    whitelisted[_users[i]] = _isWhitelisted;

    unchecked { ++i; }
}

Other instances: Gravity.sol#L233 Gravity.sol#L263 Gravity.sol#L453 Gravity.sol#L660