Byzantine Finance Atlas.sol

Description

Atlas.sol contract doesn't have a receive or fallback function, so once the delegation is active, user will not be accept ether during execute and call / batch will revert.

Similarly, the Altas delegation cannot receive ERC721 and ERC1155 tokens as onERC721Received and onERC1155Received / onERC1155BatchReceived are missing.

Proof of Concept

• add this ether sender contract

contract SomeSenderContract {    receive() external payable {}
    function sendEther(address to, uint256 amount) externa {        (bool success,) = to.call{value: amount}("");        require(success, "failed to send ether");    }}

• run this test case

function test_cannotReceiveEth() public {        SomeSenderContract senderImpl = new SomeSenderContract();        Atlas atlasImpl = new Atlas();        uint256 fundsToSend = 0.1 ether;        uint256 aliceInitialBalance = alice.addr.balance;
        vm.deal(bob.addr, 1 ether);        vm.deal(address(senderImpl), fundsToSend);
        Vm.SignedDelegation memory delegation = vm.signDelegation(address(atlasImpl), alice.privateKey);
        vm.startBroadcast(bob.privateKey);        vm.attachDelegation(delegation);        assertGt(alice.addr.code.length, 0, "Alice should have delegation code");
        Atlas.Call memory call = IAtlas.Call({            to: address(senderImpl),            value: 0,            data: abi.encodeCall(senderImpl.sendEther, (alice.addr, fundsToSend))        });
        uint256 deadline = block.timestamp + 10_000;        uint256 cnonce = vm.randomUint();
        bytes32 digest = getDigest(call, deadline, cnonce);        (uint8 v, bytes32 r, bytes32 s) = vm.sign(alice, digest);
        Atlas(alice.addr).executeCall(call, deadline, cnonce, v, r, s); // <-- THIS REVERTS
        assertEq(alice.addr.balance, aliceInitialBalance + fundsToSend, "wrong balance");    }

Recommendation

• Implement the above-mentioned functions correctly
• or alternatively, inherit from Receiver contract and modify as per needs

Description

The usedNonces mapping uses standard storage slots, which can be corrupted when users switch EIP-7702 delegations, enabling replay of previously consumed signatures.

In EIP-7702, storage lives on the delegating EOA, not the implementation contract. When a user changes delegation (Atlas → OtherImpl → Atlas), the intermediate implementation may overwrite mapping slots for its own purposes, resetting nonce values to false. This allows previously-executed non-expired signature to be replayed.

Recommendation

Implement EIP-7201 namespaced storage as recommended by EIP-7702

/// @custom:storage-location erc7201:atlas.storage.mainstruct AtlasStorage {    mapping(uint256 => bool) usedNonces;}
// keccak256(abi.encode(uint256(keccak256("atlas.storage.main")) - 1)) & ~bytes32(uint256(0xff))bytes32 private constant ATLAS_STORAGE_LOCATION = 0x...;
function _getStorage() private pure returns (AtlasStorage storage $) {    assembly { $.slot := ATLAS_STORAGE_LOCATION }}

This places usedNonces at a collision-resistant storage location, preventing accidental overwrites by other delegation targets. However, it is possible for malicious & untrusted delegation upgrades to re-write slots adversarially.

Description

Atlas is designed as an EIP-7702 delegation target, allowing EOAs to delegate to it for batch call functionality. However, the contract does not implement the EIP-1271 isValidSignature(bytes32 hash, bytes signature) function.

When an EOA delegates to a contract via EIP-7702, that EOA now has code. Many protocols check address.code.length > 0 to determine if an address is a contract and, if so, call isValidSignature to validate signatures rather than using ecrecover. Due to this, when isValidSignature is called on Atlas, the batch execution will revert.

Recommendation

• considering adding supported implementation for EIP-1271
• document the absence of the same and warn users to not interact with any callee which might call this hook

Description

The EIP712 DOMAIN_TYPEHASH doesn't include name and version fields, due to this, certain wallet might display it opaquely or even reject it.

Recommendation

• consider adding name and version for wallet UX improvements

Description

After each call, a CallExecuted event is logged with the input data to the call. However, this data is already transparently present in the transaction data and can thus be picked up by any observer checking for logs. Meanwhile, the returndata of the call is discarded, making it difficult to retrieve. This means on the one hand a gas cost burden of emitting uninteresting data (which also tends to be quite large, as meaningful calldata on average is larger than meaningful returndata) and extra burden on observers to retrieve interesting returndata, if they care about it.

Recommendation

- (bool success,) = callItem.to.call{value: callItem.value}(callItem.data);+ (bool success, bytes memory returndata) = callItem.to.call{value: callItem.value}(callItem.data);  require(success, CallReverted());- emit CallExecuted(msg.sender, callItem.to, callItem.value, callItem.data);+ emit CallExecuted(msg.sender, callItem.to, returndata);

Description

The check recoveredAddress != address(0) is unnecessary, as it's followed by recoveredAddress == address(this). Furthermore, if using OpenZeppelin's ECDSA algorithm with recover(digest, v, r, s), the result will not be 0, as the recover() function will throw an error in this case.

Recommendation

- require(recoveredAddress != address(0) && recoveredAddress == address(this), InvalidSigner());+ require(recoveredAddress == address(this), InvalidSigner());