MELD security record

The function `redeem()` in the contract `MeldDaoERC721` only ensures that it has voted in an ongoing proposal, but does not check whether it is delegated to another NFT. As a result, the delegator cannot call the function `submitVote()` to submit the vote once the NFT which is delegated to it has been redeemed, because the transaction will be reverted by the owner check in the function `updateRedeemableTimestamp()`. To solve the issue, the delegator has to identify the redeemed NFT and call the function `removeDelegatedNft()` to remove the delegation.

The function `rescueERC721()` allows the role `DEFAULT_ADMIN_ROLE` to transfer all types of the `ERC721` tokens including the current `MeldDaoERC721` tokens to an arbitrary address. The function does not check whether the `_token` address equals `address(this)`.

The contract `BaseToken` adopts the eip-2771 standard which overrides the function `_msgSender()` and uses the last 20 bytes of the call data instead of `msg.sender` if the `msg.sender` is the forwarder. Meanwhile, the function `_checkRole()` in the OpenZeppelin contract `AccessControl` is using the function `_msgSender()` for the role checks. Thus, the forwarder can forge the value of _msgSender() and effectively send transactions from privileged addresses.

The function `removeAllDelegatedNfts()` does not clear the `delegatedIndexes` as the function `removeDelegatedNft()`. The behaviors should be consistent for both functions, the only difference should be execution to all delegations. ```solidity nft.delegatedIndexes[delegatedNftId] = 0; ``` As a result, the mapping `nft.delegatedIndexes` will contain obsolete data which contradicts the mapping `nft.delegatedNfts`.

The mapping `delegatedIndexes` is the index of a given NFT in the `delegatedNfts` array. We can see that the index starts from 0 in the below code snippet in the function `delegateMyNft()`. However, this mechanism cannot distinguish the actual zero index from the null value, because the default value of the type `uint256` is also 0. If the `delegatedNfts` only has one element whose token id is 1, the value of `nft.delegatedIndexes[userNftId]` is still 0 which equals the default value. ```solidity nft.delegatedNfts.push(userNftId); nft.delegatedIndexes[userNftId] = nft.delegatedNfts.length - 1; ```

A reentrancy attack can occur when the contract creates a function that makes an external call to another untrusted contract before resolving any effects. If the attacker can control the untrusted contract, they can make a recursive call back to the original function, repeating interactions that would have otherwise not run after the external call resolved the effects. *This finding is considered informational because the meldToken is a known contract. Also, the effect of out-of-order increments may be unobservable after the transaction and the `require` statement may prevent the attack. However, the reentrancy vulnerability may still cause other issues in the middle of the transaction.* --- In the function `mint()`: ### External call(s) ```solidity=73 meldToken.safeTransferFrom(msg.sender, address(this), nftPrice); ``` ### State variables written after the call(s) ```solidity=78 lockedMeldTokens += nftPrice; ``` ```solidity=77 pricePaidPerNft[tokenId] = nftPrice; ``` ```solidity=76 _mint(msg.sender, tokenId); ``` * This function call executes the following assignment(s). * In `ERC721._mint`, * `_owners[tokenId] = to` *Note: Only a sample of 3 assignments (out of 5) are shown above.* --- In the function `redeem()`: ### External call(s) ```solidity=93 meldToken.safeTransfer(msg.sender, pricePaid - fee); ``` ```solidity=94 meldToken.safeTransfer(feeAddress, fee); ``` ### State variables written after the call(s) ```solidity=95 redeemedNfts++; ```

The variable names `index` and `nftId` in the mapping `delegatedIndexes` are reversed. The key should be `nftId` and the value should be `index`. ```solidity=24 struct Nft { bool delegateable; uint256[] delegatedNfts; mapping(uint256 index => uint256 nftId) delegatedIndexes; } ```

Functions that update state variables should emit relevant events as notifications. For example: `MeldVotingContract.startNewVotingForProposal()`.

If the wrong proposals are made, there is no cancellation function in the contracts `MeldVotingContract` and `DaoRegistry`, although the `DaoRegistry` contract mentioned the `flags` can be `canceled`(code comment L90).

The function `startNewVotingForProposal()` in the contract `MeldVotingContract` receives a parameter in `bytes` type with no name and does not use it. It might be used for recording the proposal content.