search

code-423n4 / 2021-10-ambire-findings

0 stars 0 forks source link

Incorrect checking of signature length #49

Closed code423n4 closed 3 years ago

code423n4 commented 3 years ago

Handle

JMukesh

Vulnerability details

Impact

signature which have SignatureMode.EthSign/SignatureMode.EIP712 have length 65 , so all signature coming through both mode will be reverted

https://github.com/OpenZeppelin/openzeppelin-contracts/blob/0db97c9681f488cdd739c65d911636fc6accc72c/contracts/utils/cryptography/ECDSA.sol#L56

Proof of Concept

https://github.com/code-423n4/2021-10-ambire/blob/bc01af4df3f70d1629c4e22a72c19e6a814db70d/contracts/libs/SignatureValidatorV2.sol#L42

Tools Used

manual review

Recommended Mitigation Steps

update the correct signature length

Ivshti commented 3 years ago

If this was true, this should've been high severity since it cripples the whole functionality greatly.

However, since the sigMode itself is attached to the sig, this is not actually an issue

GalloDaSballo commented 3 years ago

The sig contains the signature type at uint8(sig[sig.length - 1]) The length is increased by one because of that

Invalid finding