code-423n4 / 2021-09-defiprotocol-findings

1 stars 0 forks source link

Lack of revert messages #258

Open code423n4 opened 3 years ago

code423n4 commented 3 years ago

Handle

0xsanson

Vulnerability details

Impact

In almost all the require statements in the code the revert message is missing. This can cause issues for the users who can't determine the cause for their failing transactions.

Proof of Concept

Use grep -n 'require' *.sol for the full list.

Auction.sol:48:        require(!initialized);
Auction.sol:55:        require(auctionOngoing);
Auction.sol:56:        require(!hasBonded);
Auction.sol:76:        require(auctionOngoing);
Auction.sol:77:        require(hasBonded);
Auction.sol:78:        require(bondTimestamp + ONE_DAY > block.number);
Auction.sol:79:        require(msg.sender == auctionBonder);
Auction.sol:98:            require(IERC20(pendingTokens[i]).balanceOf(address(basket)) >= tokensNeeded);
Auction.sol:112:        require(auctionOngoing);
Auction.sol:113:        require(hasBonded);
Auction.sol:114:        require(bondTimestamp + ONE_DAY <= block.number);
Auction.sol:144:            require(bounty.active);
Basket.sol:54:        require(_tokens.length == _weights.length);
Basket.sol:61:            require(_tokens[i] != address(0));
Basket.sol:62:            require(_weights[i] > 0);
Basket.sol:65:                require(_tokens[i] != tokenList[x]);
Basket.sol:77:        require(auction.auctionOngoing() == false);
Basket.sol:78:        require(amount > 0);
Basket.sol:90:        require(auction.auctionOngoing() == false);
Basket.sol:91:        require(amount > 0);
Basket.sol:92:        require(balanceOf(msg.sender) >= amount);
Basket.sol:134:        require(newPublisher != address(0));
Basket.sol:137:            require(pendingPublisher.publisher == newPublisher);
Basket.sol:138:            require(block.number >= pendingPublisher.block + TIMELOCK_DURATION);
Basket.sol:153:        require(newLicenseFee >= factory.minLicenseFee() && newLicenseFee != licenseFee);
Basket.sol:155:            require(pendingLicenseFee.licenseFee == newLicenseFee);
Basket.sol:156:            require(block.number >= pendingLicenseFee.block + TIMELOCK_DURATION);
Basket.sol:174:            require(block.number >= pendingWeights.block + TIMELOCK_DURATION);
Basket.sol:208:        require(msg.sender == publisher || msg.sender == address(auction));
Basket.sol:209:        require(auction.auctionOngoing() == false);
Basket.sol:245:        require(msg.sender == address(auction));
Basket.sol:250:        require(msg.sender == address(publisher));
Factory.sol:56:        require(newOwnerSplit <= 2e17); // 20%
Factory.sol:74:        require(licenseFee >= minLicenseFee);
Factory.sol:95:        require(bProposal.basket == address(0));

Tools Used

grep

Recommended Mitigation Steps

Consider adding a simple message in all require statements.

frank-beard commented 2 years ago

not an exploit

GalloDaSballo commented 2 years ago

Informational finding, personal would even set to invalid, however sponsor has acknowledged, will downgrade to non-critical