EIP-2938 Breaks Whitelist Logic

[code423n4]

Lines of code

https://github.com/code-423n4/2023-08-dopex/blob/main/contracts/perp-vault/PerpetualAtlanticVault.sol#L324 https://github.com/code-423n4/2023-08-dopex/blob/main/contracts/perp-vault/PerpetualAtlanticVault.sol#L374 https://github.com/code-423n4/2023-08-dopex/blob/main/contracts/perp-vault/PerpetualAtlanticVault.sol#L409 https://github.com/code-423n4/2023-08-dopex/blob/main/contracts/core/RdpxV2Core.sol#L1055 https://github.com/code-423n4/2023-08-dopex/blob/main/contracts/core/RdpxV2Core.sol#L1086

Vulnerability details

Impact

Unauthorized contracts can bypass whitelistedContracts[msg.sender] due to EIP-2938.

Proof of Concept

In the function _isEligibleSender() it checks if (msg.sender != tx.origin){...} but when EIP-2938 (a.k.a Account Abstraction) is fully implemented it will be possible to initiate transactions with contracts as they will be top-level accounts, just like EOAs. So the problem is when the function _isEligibleSender() checks if msg.sender != tx.origin all the txs initiated by non whitelisted contracts will be able to bypass it without reverting, the function is so sure no one can by pass it that it doesn't even revert (this is expected because at the moment contracts can't initiate txs).

The point is that when the tx.origin is a contract and can initiate txs it will be able to bypass whitelistedContracts[msg.sender] without being whitelisted.

here it's the EIP for more information: https://eips.ethereum.org/EIPS/eip-2938 (not necessarily this one but the core idea is in there, the idea of making contracts initiate tx is being implemented)

Tools Used

Manual

Recommended Mitigation Steps

1 - add revert logic for unauthorized contracts. 2 - Update _isEligibleSender() to account for EIP-2938 changes.

Assessed type

Access Control

[c4-pre-sort]

bytes032 marked the issue as primary issue

[bytes032]

• Kind of related to #583
• Overinflated severity

[c4-pre-sort]

bytes032 marked the issue as low quality report

[c4-judge]

GalloDaSballo marked the issue as unsatisfactory: Overinflated severity

[GalloDaSballo]

Imo QA and as such over inflated