code-423n4 / 2021-11-badgerzaps-findings

0 stars 0 forks source link

Missing events for critical operations #55

Open code423n4 opened 3 years ago

code423n4 commented 3 years ago

Handle

WatchPug

Vulnerability details

Across the contracts, there are certain critical operations that change critical values that affect the users of the protocol.

It's a best practice for these setter functions to emit events to record these changes on-chain for off-chain monitors/tools/interfaces to register the updates and react if necessary.

Instances include:

https://github.com/Badger-Finance/ibbtc/blob/d8b95e8d145eb196ba20033267a9ba43a17be02c/contracts/Zap.sol#L322-L324

function setGovernance(address _governance) external onlyGovernance {
    _setGovernance(_governance);
}

https://github.com/Badger-Finance/ibbtc/blob/d8b95e8d145eb196ba20033267a9ba43a17be02c/contracts/Zap.sol#L331-L333

function approveContractAccess(address account) external onlyGovernance {
    _approveContractAccess(account);
}

https://github.com/Badger-Finance/ibbtc/blob/d8b95e8d145eb196ba20033267a9ba43a17be02c/contracts/Zap.sol#L335-L337

function revokeContractAccess(address account) external onlyGovernance {
    _revokeContractAccess(account);
}

https://github.com/Badger-Finance/badger-ibbtc-utility-zaps/blob/6f700995129182fec81b772f97abab9977b46026/contracts/IbbtcVaultZap.sol#L116-L119

function setGuardian(address _guardian) external {
    _onlyGovernance();
    governance = _guardian;
}

https://github.com/Badger-Finance/badger-ibbtc-utility-zaps/blob/6f700995129182fec81b772f97abab9977b46026/contracts/IbbtcVaultZap.sol#L121-L124

function setGovernance(address _governance) external {
    _onlyGovernance();
    governance = _governance;
}

https://github.com/Badger-Finance/badger-ibbtc-utility-zaps/blob/a5c71b72222d84b6414ca0339ed1761dc79fe56e/contracts/SettToRenIbbtcZap.sol#L130-L133

function setGuardian(address _guardian) external {
    _onlyGovernance();
    governance = _guardian;
}

https://github.com/Badger-Finance/badger-ibbtc-utility-zaps/blob/a5c71b72222d84b6414ca0339ed1761dc79fe56e/contracts/SettToRenIbbtcZap.sol#L135-L138

function setGovernance(address _governance) external {
    _onlyGovernance();
    governance = _governance;
}
tabshaikh commented 3 years ago

fixed

GalloDaSballo commented 3 years ago

Disagree with severity, missing events, so what You can listen to the function calls, even theGraph has functionHandlers to check for function calls and they are supported by all web3 libraries

0xleastwood commented 2 years ago

Agree with sponsor, marking all missing events issues as non-critical.