Skip to content

Commit

Permalink
feat(distribution): fix vulnerabilities
Browse files Browse the repository at this point in the history
  • Loading branch information
robertu7 committed Dec 25, 2023
1 parent ea3e38d commit 6fe6d4d
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 23 deletions.
26 changes: 13 additions & 13 deletions .gas-snapshot
Original file line number Diff line number Diff line change
Expand Up @@ -70,22 +70,22 @@ CurationTest:testCannotCurateNativeTokenZeroAddress() (gas: 16488)
CurationTest:testERC20Curation() (gas: 59908)
CurationTest:testNativeTokenCuration() (gas: 60085)
CurationTest:testNativeTokenCurationToContractAcceptor() (gas: 37466)
DistributionTest:testCannotClaimIfAlreadyClaimed() (gas: 284072)
DistributionTest:testCannotClaimIfInsufficientBalance() (gas: 391953)
DistributionTest:testCannotClaimIfInvalidProof() (gas: 244569)
DistributionTest:testCannotClaimIfInvalidTreeId() (gas: 243424)
DistributionTest:testCannotClaimIfAlreadyClaimed() (gas: 284069)
DistributionTest:testCannotClaimIfInsufficientBalance() (gas: 393827)
DistributionTest:testCannotClaimIfInvalidProof() (gas: 244573)
DistributionTest:testCannotClaimIfInvalidTreeId() (gas: 243432)
DistributionTest:testCannotDropByAttacker() (gas: 11045)
DistributionTest:testCannotDropIfInsufficientAllowance(uint256) (runs: 256, μ: 158715, ~: 158729)
DistributionTest:testCannotDropIfInsufficientBalance(uint256) (runs: 256, μ: 160955, ~: 161196)
DistributionTest:testCannotDropIfInsufficientAllowance(uint256) (runs: 256, μ: 213134, ~: 213153)
DistributionTest:testCannotDropIfInsufficientBalance(uint256) (runs: 256, μ: 215359, ~: 215614)
DistributionTest:testCannotDropIfZeroAmount() (gas: 150081)
DistributionTest:testCannotSetAdminByAdmin() (gas: 17342)
DistributionTest:testCannotSetAdminByAdmin() (gas: 17377)
DistributionTest:testCannotSetAdminByAttacker() (gas: 11111)
DistributionTest:testCannotSweepByAttacker() (gas: 228843)
DistributionTest:testCannotSweepIfZeroBalance() (gas: 230636)
DistributionTest:testClaim() (gas: 410328)
DistributionTest:testDrop() (gas: 354674)
DistributionTest:testSetAdmin() (gas: 20182)
DistributionTest:testSweep() (gas: 250986)
DistributionTest:testCannotSweepByAttacker() (gas: 228850)
DistributionTest:testCannotSweepIfZeroBalance() (gas: 230643)
DistributionTest:testClaim() (gas: 410304)
DistributionTest:testDrop() (gas: 354688)
DistributionTest:testSetAdmin() (gas: 20217)
DistributionTest:testSweep() (gas: 251040)
LogbookNFTSVGTest:testTokenURI(uint8,uint8,uint16) (runs: 256, μ: 2019505, ~: 1310779)
LogbookTest:testClaim() (gas: 135608)
LogbookTest:testDonate(uint96) (runs: 256, μ: 155485, ~: 156936)
Expand Down
1 change: 1 addition & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ analyze-logbook :; slither src/Logbook/Logbook.sol
analyze-the-space :; slither src/TheSpace/TheSpace.sol
analyze-curation :; slither src/Curation/Curation.sol
analyze-billboard :; slither src/Billboard/Billboard.sol
analyze-billboard-distribution :; slither src/Billboard/Distribution.sol

# Deployments
## Logbook
Expand Down
23 changes: 13 additions & 10 deletions src/Billboard/Distribution.sol
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ contract Distribution is IDistribution, Ownable {

constructor(address token_, address admin_) {
require(token_ != address(0), "Zero address");
require(admin_ != address(0), "Zero address");

admin = admin_;
token = token_;
Expand All @@ -44,6 +45,8 @@ contract Distribution is IDistribution, Ownable {

/// @inheritdoc IDistribution
function setAdmin(address account_) external onlyOwner {
require(account_ != address(0), "Zero address");

address _prevAdmin = admin;
admin = account_;
emit AdminChanged(_prevAdmin, admin);
Expand All @@ -57,18 +60,18 @@ contract Distribution is IDistribution, Ownable {
function drop(bytes32 merkleRoot_, uint256 amount_) external payable isFromAdmin returns (uint256 treeId_) {
require(amount_ > 0, "Zero amount");

// Transfer
SafeERC20.safeTransferFrom(IERC20(token), msg.sender, address(this), amount_);

// Set the merkle root
lastTreeId.increment();
treeId_ = lastTreeId.current();
merkleRoots[treeId_] = merkleRoot_;

emit Drop(treeId_, amount_);

// Set the balance for the tree
balances[treeId_] = amount_;

emit Drop(treeId_, amount_);
// Transfer
SafeERC20.safeTransferFrom(IERC20(token), msg.sender, address(this), amount_);
}

/// @inheritdoc IDistribution
Expand All @@ -91,13 +94,13 @@ contract Distribution is IDistribution, Ownable {
// Mark it as claimed first for to prevent reentrancy
hasClaimed[treeId_][cid_][account_] = true;

// Transfer
require(IERC20(token).transfer(account_, amount_), "Failed token transfer");
emit Claim(cid_, account_, amount_);

// Update the balance for the tree
balances[treeId_] -= amount_;

emit Claim(cid_, account_, amount_);
// Transfer
require(IERC20(token).transfer(account_, amount_), "Failed token transfer");
}

/// @inheritdoc IDistribution
Expand All @@ -106,10 +109,10 @@ contract Distribution is IDistribution, Ownable {

require(_balance > 0, "Zero balance");

// Transfer
require(IERC20(token).transfer(target_, _balance), "Failed token transfer");

// Update the balance for the tree
balances[treeId_] = 0;

// Transfer
require(IERC20(token).transfer(target_, _balance), "Failed token transfer");
}
}

0 comments on commit 6fe6d4d

Please sign in to comment.