Airdrop audit fixes (#634)

* [Q-2] Make processed mapping public

* [Q-3] Unused error definition

* [Q-4] Use separate events for each airdrop type

* [Q-5] Use safeTransferETH instead of low-level call

* remove receive and withdraw

* cleanup

* [Q-1] Missing natspec documentation

* [L-1] Airdropping tokens using push or signature-based methods can be griefed

* [G-1] Use verifyCalldata to verify Merkle tree

* support EIP1271 signatures

* revert L-1

* The statement using ECDSA for bytes32 is not needed anymore

Author: kumaryash90

contracts/prebuilts/unaudited/airdrop/Airdrop.sol

@@ -16,6 +16,7 @@ import "@solady/src/utils/MerkleProofLib.sol";
 import "@solady/src/utils/ECDSA.sol";
 import "@solady/src/utils/EIP712.sol";
 import "@solady/src/utils/SafeTransferLib.sol";
+import "@solady/src/utils/SignatureCheckerLib.sol";
 
 import { Initializable } from "../../../extension/Initializable.sol";
 import { Ownable } from "../../../extension/Ownable.sol";
@@ -25,8 +26,6 @@ import "../../../eip/interface/IERC721.sol";
 import "../../../eip/interface/IERC1155.sol";
 
 contract Airdrop is EIP712, Initializable, Ownable {
-    using ECDSA for bytes32;
-
     /*///////////////////////////////////////////////////////////////
                             State, constants & structs
     //////////////////////////////////////////////////////////////*/
@@ -38,7 +37,7 @@ contract Airdrop is EIP712, Initializable, Ownable {
     /// @dev conditionId => hash(claimer address, token address, token id [1155]) => has claimed
     mapping(uint256 => mapping(bytes32 => bool)) private claimed;
     /// @dev Mapping from request UID => whether the request is processed.
-    mapping(bytes32 => bool) private processed;
+    mapping(bytes32 => bool) public processed;
 
     struct AirdropContentERC20 {
         address recipient;
@@ -106,19 +105,18 @@ contract Airdrop is EIP712, Initializable, Ownable {
 
     error AirdropInvalidProof();
     error AirdropAlreadyClaimed();
-    error AirdropFailed();
     error AirdropNoMerkleRoot();
     error AirdropValueMismatch();
     error AirdropRequestExpired(uint256 expirationTimestamp);
     error AirdropRequestAlreadyProcessed();
     error AirdropRequestInvalidSigner();
-    error AirdropInvalidTokenAddress();
 
     /*///////////////////////////////////////////////////////////////
                                 Events
     //////////////////////////////////////////////////////////////*/
 
     event Airdrop(address token);
+    event AirdropWithSignature(address token);
     event AirdropClaimed(address token, address receiver);
 
     /*///////////////////////////////////////////////////////////////
@@ -133,34 +131,25 @@ contract Airdrop is EIP712, Initializable, Ownable {
         _setupOwner(_defaultAdmin);
     }
 
-    /*///////////////////////////////////////////////////////////////
-                        Receive and withdraw logic
-    //////////////////////////////////////////////////////////////*/
-
-    receive() external payable {}
-
-    function withdraw(address _tokenAddress, uint256 _amount) external onlyOwner {
-        if (_tokenAddress == NATIVE_TOKEN_ADDRESS) {
-            SafeTransferLib.safeTransferETH(msg.sender, _amount);
-        } else {
-            SafeTransferLib.safeTransferFrom(_tokenAddress, address(this), msg.sender, _amount);
-        }
-    }
-
     /*///////////////////////////////////////////////////////////////
                             Airdrop Push
     //////////////////////////////////////////////////////////////*/
 
+    /**
+     *  @notice          Lets contract-owner send native token (eth) to a list of addresses.
+     *  @dev             Owner should send total airdrop amount as msg.value.
+     *                   Can only be called by contract owner.
+     *
+     *  @param _contents List containing recipients and amounts to airdrop
+     */
     function airdropNativeToken(AirdropContentERC20[] calldata _contents) external payable onlyOwner {
         uint256 len = _contents.length;
         uint256 nativeTokenAmount;
 
         for (uint256 i = 0; i < len; i++) {
             nativeTokenAmount += _contents[i].amount;
-            (bool success, ) = _contents[i].recipient.call{ value: _contents[i].amount }("");
-            if (!success) {
-                revert AirdropFailed();
-            }
+
+            SafeTransferLib.safeTransferETH(_contents[i].recipient, _contents[i].amount);
         }
 
         if (nativeTokenAmount != msg.value) {
@@ -170,6 +159,14 @@ contract Airdrop is EIP712, Initializable, Ownable {
         emit Airdrop(NATIVE_TOKEN_ADDRESS);
     }
 
+    /**
+     *  @notice          Lets contract owner send ERC20 tokens to a list of addresses.
+     *  @dev             The token-owner should approve total airdrop amount to this contract.
+     *                   Can only be called by contract owner.
+     *
+     *  @param _tokenAddress Address of the ERC20 token being airdropped
+     *  @param _contents     List containing recipients and amounts to airdrop
+     */
     function airdropERC20(address _tokenAddress, AirdropContentERC20[] calldata _contents) external onlyOwner {
         uint256 len = _contents.length;
 
@@ -180,6 +177,14 @@ contract Airdrop is EIP712, Initializable, Ownable {
         emit Airdrop(_tokenAddress);
     }
 
+    /**
+     *  @notice          Lets contract owner send ERC721 tokens to a list of addresses.
+     *  @dev             The token-owner should approve airdrop tokenIds to this contract.
+     *                   Can only be called by contract owner.
+     *
+     *  @param _tokenAddress Address of the ERC721 token being airdropped
+     *  @param _contents     List containing recipients and tokenIds to airdrop
+     */
     function airdropERC721(address _tokenAddress, AirdropContentERC721[] calldata _contents) external onlyOwner {
         uint256 len = _contents.length;
 
@@ -190,6 +195,14 @@ contract Airdrop is EIP712, Initializable, Ownable {
         emit Airdrop(_tokenAddress);
     }
 
+    /**
+     *  @notice          Lets contract owner send ERC1155 tokens to a list of addresses.
+     *  @dev             The token-owner should approve airdrop tokenIds and amounts to this contract.
+     *                   Can only be called by contract owner.
+     *
+     *  @param _tokenAddress Address of the ERC1155 token being airdropped
+     *  @param _contents     List containing recipients, tokenIds, and amounts to airdrop
+     */
     function airdropERC1155(address _tokenAddress, AirdropContentERC1155[] calldata _contents) external onlyOwner {
         uint256 len = _contents.length;
 
@@ -210,6 +223,14 @@ contract Airdrop is EIP712, Initializable, Ownable {
                         Airdrop With Signature
     //////////////////////////////////////////////////////////////*/
 
+    /**
+     *  @notice          Lets contract owner send ERC20 tokens to a list of addresses with EIP-712 signature.
+     *  @dev             The token-owner should approve airdrop amounts to this contract.
+     *                   Signer should be the contract owner.
+     *
+     *  @param req          Struct containing airdrop contents, uid, and expiration timestamp
+     *  @param signature    EIP-712 signature to perform the airdrop
+     */
     function airdropERC20WithSignature(AirdropRequestERC20 calldata req, bytes calldata signature) external {
         // verify expiration timestamp
         if (req.expirationTimestamp < block.timestamp) {
@@ -239,9 +260,17 @@ contract Airdrop is EIP712, Initializable, Ownable {
             );
         }
 
-        emit Airdrop(req.tokenAddress);
+        emit AirdropWithSignature(req.tokenAddress);
     }
 
+    /**
+     *  @notice          Lets contract owner send ERC721 tokens to a list of addresses with EIP-712 signature.
+     *  @dev             The token-owner should approve airdrop tokenIds to this contract.
+     *                   Signer should be the contract owner.
+     *
+     *  @param req          Struct containing airdrop contents, uid, and expiration timestamp
+     *  @param signature    EIP-712 signature to perform the airdrop
+     */
     function airdropERC721WithSignature(AirdropRequestERC721 calldata req, bytes calldata signature) external {
         // verify expiration timestamp
         if (req.expirationTimestamp < block.timestamp) {
@@ -266,9 +295,17 @@ contract Airdrop is EIP712, Initializable, Ownable {
             IERC721(req.tokenAddress).safeTransferFrom(_from, req.contents[i].recipient, req.contents[i].tokenId);
         }
 
-        emit Airdrop(req.tokenAddress);
+        emit AirdropWithSignature(req.tokenAddress);
     }
 
+    /**
+     *  @notice          Lets contract owner send ERC1155 tokens to a list of addresses with EIP-712 signature.
+     *  @dev             The token-owner should approve airdrop tokenIds and amounts to this contract.
+     *                   Signer should be the contract owner.
+     *
+     *  @param req          Struct containing airdrop contents, uid, and expiration timestamp
+     *  @param signature    EIP-712 signature to perform the airdrop
+     */
     function airdropERC1155WithSignature(AirdropRequestERC1155 calldata req, bytes calldata signature) external {
         // verify expiration timestamp
         if (req.expirationTimestamp < block.timestamp) {
@@ -299,13 +336,23 @@ contract Airdrop is EIP712, Initializable, Ownable {
             );
         }
 
-        emit Airdrop(req.tokenAddress);
+        emit AirdropWithSignature(req.tokenAddress);
     }
 
     /*///////////////////////////////////////////////////////////////
                             Airdrop Claimable
     //////////////////////////////////////////////////////////////*/
 
+    /**
+     *  @notice          Lets allowlisted addresses claim ERC20 airdrop tokens.
+     *  @dev             The token-owner should approve total airdrop amount to this contract,
+     *                   and set merkle root of allowlisted address for this airdrop.
+     *
+     *  @param _token       Address of ERC20 airdrop token
+     *  @param _receiver    Allowlisted address for which the token is being claimed
+     *  @param _quantity    Allowlisted quantity of tokens to claim
+     *  @param _proofs      Merkle proofs for allowlist verification
+     */
     function claimERC20(address _token, address _receiver, uint256 _quantity, bytes32[] calldata _proofs) external {
         bytes32 claimHash = _getClaimHashERC20(_receiver, _token);
         uint256 conditionId = tokenConditionId[_token];
@@ -319,7 +366,7 @@ contract Airdrop is EIP712, Initializable, Ownable {
             revert AirdropNoMerkleRoot();
         }
 
-        bool valid = MerkleProofLib.verify(
+        bool valid = MerkleProofLib.verifyCalldata(
             _proofs,
             _tokenMerkleRoot,
             keccak256(abi.encodePacked(_receiver, _quantity))
@@ -335,6 +382,16 @@ contract Airdrop is EIP712, Initializable, Ownable {
         emit AirdropClaimed(_token, _receiver);
     }
 
+    /**
+     *  @notice          Lets allowlisted addresses claim ERC721 airdrop tokens.
+     *  @dev             The token-owner should approve airdrop tokenIds to this contract,
+     *                   and set merkle root of allowlisted address for this airdrop.
+     *
+     *  @param _token       Address of ERC721 airdrop token
+     *  @param _receiver    Allowlisted address for which the token is being claimed
+     *  @param _tokenId     Allowlisted tokenId to claim
+     *  @param _proofs      Merkle proofs for allowlist verification
+     */
     function claimERC721(address _token, address _receiver, uint256 _tokenId, bytes32[] calldata _proofs) external {
         bytes32 claimHash = _getClaimHashERC721(_receiver, _token, _tokenId);
         uint256 conditionId = tokenConditionId[_token];
@@ -348,7 +405,11 @@ contract Airdrop is EIP712, Initializable, Ownable {
             revert AirdropNoMerkleRoot();
         }
 
-        bool valid = MerkleProofLib.verify(_proofs, _tokenMerkleRoot, keccak256(abi.encodePacked(_receiver, _tokenId)));
+        bool valid = MerkleProofLib.verifyCalldata(
+            _proofs,
+            _tokenMerkleRoot,
+            keccak256(abi.encodePacked(_receiver, _tokenId))
+        );
         if (!valid) {
             revert AirdropInvalidProof();
         }
@@ -360,6 +421,17 @@ contract Airdrop is EIP712, Initializable, Ownable {
         emit AirdropClaimed(_token, _receiver);
     }
 
+    /**
+     *  @notice          Lets allowlisted addresses claim ERC1155 airdrop tokens.
+     *  @dev             The token-owner should approve tokenIds and total airdrop amounts to this contract,
+     *                   and set merkle root of allowlisted address for this airdrop.
+     *
+     *  @param _token       Address of ERC1155 airdrop token
+     *  @param _receiver    Allowlisted address for which the token is being claimed
+     *  @param _tokenId     Allowlisted tokenId to claim
+     *  @param _quantity    Allowlisted quantity of tokens to claim
+     *  @param _proofs      Merkle proofs for allowlist verification
+     */
     function claimERC1155(
         address _token,
         address _receiver,
@@ -379,7 +451,7 @@ contract Airdrop is EIP712, Initializable, Ownable {
             revert AirdropNoMerkleRoot();
         }
 
-        bool valid = MerkleProofLib.verify(
+        bool valid = MerkleProofLib.verifyCalldata(
             _proofs,
             _tokenMerkleRoot,
             keccak256(abi.encodePacked(_receiver, _tokenId, _quantity))
@@ -399,6 +471,13 @@ contract Airdrop is EIP712, Initializable, Ownable {
                             Setter functions
     //////////////////////////////////////////////////////////////*/
 
+    /**
+     *  @notice          Lets contract owner set merkle root (allowlist) for claim based airdrops.
+     *
+     *  @param _token             Address of airdrop token
+     *  @param _tokenMerkleRoot   Merkle root of allowlist
+     *  @param _resetClaimStatus  Reset claim status / amount claimed so far to zero for all recipients
+     */
     function setMerkleRoot(address _token, bytes32 _tokenMerkleRoot, bool _resetClaimStatus) external onlyOwner {
         if (_resetClaimStatus || tokenConditionId[_token] == 0) {
             tokenConditionId[_token] += 1;
@@ -410,6 +489,7 @@ contract Airdrop is EIP712, Initializable, Ownable {
                         Miscellaneous
     //////////////////////////////////////////////////////////////*/
 
+    /// @notice Returns claim status of a receiver for a claim based airdrop
     function isClaimed(address _receiver, address _token, uint256 _tokenId) external view returns (bool) {
         uint256 _conditionId = tokenConditionId[_token];
 
@@ -425,28 +505,33 @@ contract Airdrop is EIP712, Initializable, Ownable {
 
         return false;
     }
-
+    /// @dev Checks whether contract owner can be set in the given execution context.
     function _canSetOwner() internal view virtual override returns (bool) {
         return msg.sender == owner();
     }
 
+    /// @dev Domain name and version for EIP-712
     function _domainNameAndVersion() internal pure override returns (string memory name, string memory version) {
         name = "Airdrop";
         version = "1";
     }
 
+    /// @dev Keccak256 hash of receiver and token addresses, for claim based airdrop status tracking
     function _getClaimHashERC20(address _receiver, address _token) private view returns (bytes32) {
         return keccak256(abi.encodePacked(_receiver, _token));
     }
 
+    /// @dev Keccak256 hash of receiver, token address and tokenId, for claim based airdrop status tracking
     function _getClaimHashERC721(address _receiver, address _token, uint256 _tokenId) private view returns (bytes32) {
         return keccak256(abi.encodePacked(_receiver, _token, _tokenId));
     }
 
+    /// @dev Keccak256 hash of receiver, token address and tokenId, for claim based airdrop status tracking
     function _getClaimHashERC1155(address _receiver, address _token, uint256 _tokenId) private view returns (bytes32) {
         return keccak256(abi.encodePacked(_receiver, _token, _tokenId));
     }
 
+    /// @dev Hash nested struct within AirdropRequest___
     function _hashContentInfoERC20(AirdropContentERC20[] calldata contents) private pure returns (bytes32) {
         bytes32[] memory contentHashes = new bytes32[](contents.length);
         for (uint256 i = 0; i < contents.length; i++) {
@@ -455,6 +540,7 @@ contract Airdrop is EIP712, Initializable, Ownable {
         return keccak256(abi.encodePacked(contentHashes));
     }
 
+    /// @dev Hash nested struct within AirdropRequest___
     function _hashContentInfoERC721(AirdropContentERC721[] calldata contents) private pure returns (bytes32) {
         bytes32[] memory contentHashes = new bytes32[](contents.length);
         for (uint256 i = 0; i < contents.length; i++) {
@@ -465,6 +551,7 @@ contract Airdrop is EIP712, Initializable, Ownable {
         return keccak256(abi.encodePacked(contentHashes));
     }
 
+    /// @dev Hash nested struct within AirdropRequest___
     function _hashContentInfoERC1155(AirdropContentERC1155[] calldata contents) private pure returns (bytes32) {
         bytes32[] memory contentHashes = new bytes32[](contents.length);
         for (uint256 i = 0; i < contents.length; i++) {
@@ -475,6 +562,7 @@ contract Airdrop is EIP712, Initializable, Ownable {
         return keccak256(abi.encodePacked(contentHashes));
     }
 
+    /// @dev Verify EIP-712 signature
     function _verifyRequestSignerERC20(
         AirdropRequestERC20 calldata req,
         bytes calldata signature
@@ -485,10 +573,11 @@ contract Airdrop is EIP712, Initializable, Ownable {
         );
 
         bytes32 digest = _hashTypedData(structHash);
-        address recovered = digest.recover(signature);
-        return recovered == owner();
+
+        return SignatureCheckerLib.isValidSignatureNowCalldata(owner(), digest, signature);
     }
 
+    /// @dev Verify EIP-712 signature
     function _verifyRequestSignerERC721(
         AirdropRequestERC721 calldata req,
         bytes calldata signature
@@ -499,10 +588,11 @@ contract Airdrop is EIP712, Initializable, Ownable {
         );
 
         bytes32 digest = _hashTypedData(structHash);
-        address recovered = digest.recover(signature);
-        return recovered == owner();
+
+        return SignatureCheckerLib.isValidSignatureNowCalldata(owner(), digest, signature);
     }
 
+    /// @dev Verify EIP-712 signature
     function _verifyRequestSignerERC1155(
         AirdropRequestERC1155 calldata req,
         bytes calldata signature
@@ -513,7 +603,7 @@ contract Airdrop is EIP712, Initializable, Ownable {
         );
 
         bytes32 digest = _hashTypedData(structHash);
-        address recovered = digest.recover(signature);
-        return recovered == owner();
+
+        return SignatureCheckerLib.isValidSignatureNowCalldata(owner(), digest, signature);
     }
 }