DeFiFoundry
60,000 USDC
View results
Submission Details
Severity: low
Invalid

Some users would not be able to fill orders

Summary

Inorder to fill offchain orders, Zaros uses EIP 712 to make and verify these signatures, which would not work completely for the intended use, considering Zaros should expect contracts to make these signatures (not just EOAs), but EIP712 does not work well with smart contracts or wallets and as such only EOAs integrating with Zaros are ensured to be able to migrate, where as other (non-EOA) users are not.

Vulnerability Details

Take a look at https://github.com/Cyfrin/2024-07-zaros/blob/d687fe96bb7ace8652778797052a38763fbcbb1b/src/perpetuals/branches/SettlementBranch.sol#L260-L263

function fillOffchainOrders(){
..snip
// `ecrecover`s the order signer.
ctx.signer = ECDSA.recover(
_hashTypedDataV4(ctx.structHash), ctx.offchainOrder.v, ctx.offchainOrder.r, ctx.offchainOrder.s
);
..snip
}

This function is used to fill pending, eligible offchain offchain orders targeting the given market id.

Note that these signatures are expected to be verified for hashed data and are gotten with the help of EIP712.

function _hashTypedDataV4(bytes32 structHash) internal view virtual returns (bytes32) {
return MessageHashUtils.toTypedDataHash(_domainSeparatorV4(), structHash);
}

Problem however is that some users would not be able to access this functionality as valid signatures from them wouldn't work since they can't work with EIP 712.

According to EIP1271: Standard Signature Validation Method for Contracts:

Externally Owned Accounts (EOA) can sign messages with their associated private keys, but currently contracts cannot. We propose a standard way for any contracts to verify whether a signature on a behalf of a given contract is valid. This is possible via the implementation of a isValidSignature(hash, signature) function on the signing contract, which can be called to validate a signature.

So while recovering a valid message signed by these set of users , the return value will be the bytes4(0) for any vote signed by a contract (e.g. Multisig) because contracts that sign messages sticking to the EIP1271 standard use the EIP1271_MAGIC_VALUE as the successful return for a properly recovered signature. A sample of this is shown within the EIP1271 and also within CompatibilityFallbackHandler by GnosisSafe.

As a result of this scenario, these set of users would not be fill any order.

Impact

Some users would not be able to fill orders

Tools Used

Manual review

Recommendations

Consider adding contract signature support by implementing a recovery via the suggested isValidSignature() function of the EIP1271 and comparing the recovered value against the MAGIC_VALUE.

Updates

Lead Judging Commences

inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

EIP712 does not work well with smart contracts or wallets

Appeal created

bauchibred Submitter
12 months ago
inallhonesty Lead Judge
12 months ago
inallhonesty Lead Judge 11 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

EIP712 does not work well with smart contracts or wallets

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.