Cudos contest - 0x1f8b's results

Lines of code

https://github.com/code-423n4/2022-05-cudos/blob/de39cf3cd1f1e1cf211819b06d4acf6a043acda0/solidity/contracts/Gravity.sol#L185 https://github.com/code-423n4/2022-05-cudos/blob/de39cf3cd1f1e1cf211819b06d4acf6a043acda0/solidity/contracts/Gravity.sol#L238-L240

Vulnerability details

Impact

It's possible to bypass verifySig and checkValidatorSignatures methods using empty signers.

Proof of Concept

The method ecrecover returns address(0) when the signature is wrong, so if a user use address(0) as a validator or _signer the return will be true.

_signer == ecrecover(messageDigest, _v, _r, _s);

Also, the method checkValidatorSignatures never check that the validator is repeated inside the array, so if someone it's able to specify the validators, and it's able to sign with one valid key, if he repeat the same signature multiple times, it will be computed as a different one.

Recommended Mitigation Steps

check that _signer is not empty.

1. Remove storage variable MAX_UINT and use constant or inline.

• CosmosToken.sol#L5

2. There are require messages bigger than 32 bytes. More than 32 bytes for message will incur an extra gas costs.

• Gravity.sol#L119
• Gravity.sol#L291
• Gravity.sol#L312
• Gravity.sol#L386
• Gravity.sol#L392
• Gravity.sol#L407
• Gravity.sol#L418
• Gravity.sol#L496
• Gravity.sol#L511
• Gravity.sol#L655
• Gravity.sol#L668

3. Change the incremental logic from i++ to ++i in order to save some opcodes:

• Gravity.sol#L128
• Gravity.sol#L453
• Gravity.sol#L527
• Gravity.sol#L568
• Gravity.sol#L579
• Gravity.sol#L660