search

code-423n4 / 2022-04-abranft-findings

0 stars 0 forks source link

Signature Verification Functions Do Not Check Address Is Not Zero #194

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-04-abranft/blob/5cd4edc3298c05748e952f8a8c93e42f930a78c2/contracts/NFTPairWithOracle.sol#L452 https://github.com/code-423n4/2022-04-abranft/blob/5cd4edc3298c05748e952f8a8c93e42f930a78c2/contracts/NFTPairWithOracle.sol#L417 https://github.com/code-423n4/2022-04-abranft/blob/5cd4edc3298c05748e952f8a8c93e42f930a78c2/contracts/NFTPair.sol#L383 https://github.com/code-423n4/2022-04-abranft/blob/5cd4edc3298c05748e952f8a8c93e42f930a78c2/contracts/NFTPair.sol#L383

Vulnerability details

Impact

The function ecrecover() will return address(0) if the signature verification fails.

There are no checks during the signature verification to ensure the return value is non-zero. The result is that it's possible for the signature to pass if lender = address(0) / borrower = address(0)

Proof of Concept

            require(ecrecover(_getDigest(dataHash), signature.v, signature.r, signature.s) == lender, "NFTPair: signature invalid");
        require(ecrecover(_getDigest(dataHash), signature.v, signature.r, signature.s) == borrower, "NFTPair: signature invalid");

Recommended Mitigation Steps

This issue may be mitigated by ensure that the borrower / lender is a non-zero address.

cryptolyndon commented 2 years ago

Duplicate of #1, #2, #3 and #4, not necessarily in that order.

0xean commented 2 years ago

see #2 for explanation, downgrading to QA