Main Seaweed Condor - Improper and outdated check of valid signature for wallet address like Multisig wallet is there.

[sherlock-admin3]

Main Seaweed Condor

High

Improper and outdated check of valid signature for wallet address like Multisig wallet is there.

Summary

In Contract PredictDotLoan.sol, SignatureChecker of openzeppelin is used for checking if signature is valid or not. But check for contract address can be bypass and if borrower or lender is multisig wallet or contract address having ownable fucntionallity , code logic can be bypassed . As outdated verify signature check is there. With new eip 7377 implementation, there can many address having same private key as contract address having the logic

Root Cause

function _assertValidSignature(bytes32 proposalId, address from, bytes calldata signature) private view {
    if (!SignatureChecker.isValidSignatureNow(from, proposalId, signature)) {
        revert InvalidSignature();
    }
}

  function isValidSignatureNow(address signer, bytes32 hash, bytes memory signature) internal view returns (bool) {
      (address recovered, ECDSA.RecoverError error, ) = ECDSA.tryRecover(hash, signature);
return
    (error == ECDSA.RecoverError.NoError && recovered == signer) ||
    isValidERC1271SignatureNow(signer, hash, signature);
  }

https://github.com/sherlock-audit/2024-09-predict-fun/blob/41e70f9eed3f00dd29aba4038544150f5b35dccb/predict-dot-loan/contracts/PredictDotLoan.sol#L1395C5-L1399C6

This implementation assumes that ecrecover will not identify the correct signer if the signer is a smart contract. This comes from the assumption that there contract addresses is generated in such a way that there is no (known) private key that derivates to the same address.

These assumption could be challenged by EIP-7377.

If EIP-7377 is deployed, a private key would be able to deploy code at its own address. In that case, there would be a known private key for the contract. This causes a governance issue. Some people may expect the contract to have some shared ownership (Multisig), be controlled by another wallet (Ownable), or just trustless. But using the private key that was used to deploy it, the deployer could generate a Permit signature (or similar) that could result in funds being drained out of the contract.

This attack will be feasable on historical implementations if the ecrecover precompile does not check the presence of code at the recovered location

It as been proposed to not rely on the precompile possibly being changed

Internal pre-conditions

So situation could be occur when lender or borrower creates the loan with multiSig wallet and that contract is created with private so, it skips the code logic if signature is signed by private of address having same address as contract address. And all tokens can be drained out without proper logic

No response

Attack Path

No response

Impact

Signature in the proposal can be verified, without checking proper logic of the code written in code , so _assertValidSignature() put in all functions , can be tampered. so matchProposal() can also tampered. and funds transferred to this address can also drained as proper proposal validity check is not done by protocol because this check is almost in every imp function of protocol

PoC

No response

Mitigation

function isValidSignatureNow(address signer, bytes32 hash, bytes memory signature) internal view returns (bool) { if (signer.code.length == 0) { (address recovered, ECDSA.RecoverError error, ) = ECDSA.tryRecover(hash, signature); return (error == ECDSA.RecoverError.NoError && recovered == signer); } else { return isValidERC1271SignatureNow(signer, hash, signature); } }

this is implemented in latest version of openzeppelin Signature checker so it should be used.