ecrecover returns zero address for invalid signatures

Summary

When ecrecover is fed with incorrect values of v, r, s, it does not revert but returns an empty address 0x0.

Vulnerability Detail

function _validateCommitment expects a valid signature from the strategist:

    address recovered = ecrecover(
      ...
      params.lienRequest.v,
      params.lienRequest.r,
      params.lienRequest.s
    );
    require(
      recovered == params.lienRequest.strategy.strategist,
      "strategist must match signature"
    );
    require(
      recovered == owner() || recovered == delegate,
      "invalid strategist"
    );

The recovered address must either be an owner or a delegate. It is totally possible that delegate is not set (an empty address) because the protocol does not enforce that.

Impact

When the delegate address is not set, a signature check can be bypassed and malicious strategy data accepted as valid.

Code Snippet

https://github.com/sherlock-audit/2022-10-astaria/blob/main/src/VaultImplementation.sol#L167-L185

https://github.com/sherlock-audit/2022-10-astaria/blob/main/src/VaultImplementation.sol#L118-L124

https://github.com/sherlock-audit/2022-10-astaria/blob/main/src/VaultImplementation.sol#L131-L133

Tool used

Manual Review

Recommendation

require recovered is not address(0) or use OZ ECDSA library.

Duplicate of #69

sherlock-audit / 2022-10-astaria-judging

HonorLt - ecrecover returns zero address for invalid signatures #246