code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2022-12-tigris/blob/588c84b7bb354d20cbca6034544c4faa46e6a80e/contracts/GovNFT.sol#L307-L309 https://github.com/code-423n4/2022-12-tigris/blob/588c84b7bb354d20cbca6034544c4faa46e6a80e/contracts/BondNFT.sol#L349-L355

Vulnerability details

Impact

Malicious owner can deny service of whole protocol by disallowing all assets or simply not including them

Proof of Concept

Owner has to add asset in order to use the asset in BondNFT.sol

function addAsset(address _asset) external onlyOwner {
    require(assets.length == 0 || assets[assetsIndex[_asset]] != _asset, "Already added");
    assetsIndex[_asset] = assets.length;
    assets.push(_asset);
    allowedAsset[_asset] = true;
    epoch[_asset] = block.timestamp/DAY;
}

Owner can also editAsset in Lock.sol

function editAsset(
    address _tigAsset,
    bool _isAllowed
) external onlyOwner() {
    allowedAssets[_tigAsset] = _isAllowed;
}

Likewise, in GovNFT.sol, owner can setAllowedAsset

function setAllowedAsset(address _asset, bool _bool) external onlyOwner {
    _allowedAsset[_asset] = _bool;
}

Tools Used

Manual Review

Recommended Mitigation Steps

Usage of a DAO, or some assets can already be in play, or a series of multisig address to counter the centralization issue.

TriHaz commented 1 year ago

We are aware of the centralization risks, initially, all contracts will have a multi-sig as owner to prevent a sole owner, later on a DAO could be the owner.

c4-sponsor commented 1 year ago

TriHaz marked the issue as sponsor acknowledged

c4-judge commented 1 year ago

GalloDaSballo marked the issue as duplicate of #377