(Low) setXXXXXLimits, setMaxXXXXX and setXXXXXBounds functions don’t consider the original value
Impact
In set limit function (e.g. setDiscountLimits), it doesn’t consider the original value (funding.discount), resulting in the original value being out of bound.
Proof of Concept
For example, the original value funding.discount is 20, minDiscount is 10, and maxDiscount is 100. If Governance calls setDiscountLimits, changes minDiscount to 30, and changes maxDiscount to 50, the original funding.discount will be out of bound.
Summary
We list 3 low-critical findings here:
transferFromDisabledis unused_daoIdIn conclusion, it's better to check unused variables, check the validation of DAO ID, and check the boundaries of original values.
(Low)
transferFromDisabledis unusedImpact
transferFromDisabledis unused and not initialized, it may cause some problems when we need it in the future.Proof of Concept
https://github.com/code-423n4/2022-04-badger-citadel/blob/main/src/GlobalAccessControl.sol#L51
Tools Used
vim, ganache-cli
Recommended Mitigation Steps
Delete unused variables or set
transferFromDisabledininitialize().(Low) No check for
_daoIdImpact
No check for
_daoIdofbuy()function in KnightingRound.sol, resulting in a user being able to buy non-exist or invalid_daoId.Proof of Concept
https://github.com/code-423n4/2022-04-badger-citadel/blob/main/src/KnightingRound.sol#L164
Tools Used
vim, ganache-cli
Recommended Mitigation Steps
Check valid range of
_daoIdinbuy()function.(Low) setXXXXXLimits, setMaxXXXXX and setXXXXXBounds functions don’t consider the original value
Impact
In set limit function (e.g.
setDiscountLimits), it doesn’t consider the original value (funding.discount), resulting in the original value being out of bound.Proof of Concept
For example, the original value
funding.discountis 20,minDiscountis 10, andmaxDiscountis 100. If Governance callssetDiscountLimits, changesminDiscountto 30, and changesmaxDiscountto 50, the originalfunding.discountwill be out of bound.https://github.com/code-423n4/2022-04-badger-citadel/blob/main/src/Funding.sol#L356 https://github.com/code-423n4/2022-04-badger-citadel/blob/main/src/Funding.sol#L397 https://github.com/code-423n4/2022-04-badger-citadel/blob/main/src/StakedCitadel.sol#L521 https://github.com/code-423n4/2022-04-badger-citadel/blob/main/src/StakedCitadel.sol#L533 https://github.com/code-423n4/2022-04-badger-citadel/blob/main/src/StakedCitadel.sol#L548
Tools Used
vim, ganache-cli
Recommended Mitigation Steps
Check the original value in set limit functions.