Skip to content

Commit

Permalink
Airdrop audit fixes (#634)
Browse files Browse the repository at this point in the history 
* [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
  • Loading branch information
@kumaryash90
kumaryash90 authored Apr 5, 2024
1 parent a9e6477 commit 2104a1a
Show file tree
Hide file tree
Showing 4 changed files with 485 additions and 183 deletions.
162 changes: 126 additions & 36 deletions contracts/prebuilts/unaudited/airdrop/Airdrop.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand All @@ -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
//////////////////////////////////////////////////////////////*/
Expand All @@ -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;
Expand Down Expand Up @@ -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);

/*///////////////////////////////////////////////////////////////
Expand All @@ -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) {
Expand All @@ -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;

Expand All @@ -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;

Expand All @@ -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;

Expand All @@ -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) {
Expand Down Expand Up @@ -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) {
Expand All @@ -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) {
Expand Down Expand Up @@ -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];
Expand All @@ -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))
Expand All @@ -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];
Expand All @@ -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();
}
Expand All @@ -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,
Expand All @@ -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))
Expand All @@ -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;
Expand All @@ -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];

Expand All @@ -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++) {
Expand All @@ -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++) {
Expand All @@ -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++) {
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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);
}
}
Loading

0 comments on commit 2104a1a

Please sign in to comment.