simon-something
commented
2 years ago Findings are either wrongs or inaccurate, I suggest not accepting this one
1) this is the implementation of an interface 2) there are no such things as race condition in evm 3) the new token address is accessible onchain, the old token wouldn't be anymore 4) changeFor is, as suggested by the modifier, called by the controller 5) we don't implement a total supply cap 6) yes, any user can send it's own value in the mapping (we don't check for access since we rely on msg.sender) 7) complexity assessment is strongly operator-dependant
https://github.com/jbx-protocol/juice-contracts-v2-code4rena/blob/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBToken.sol#L34
Input var ( _prijec0tId) is not used in the function body. So this unused entry may confuse future readers and users of this smart contract. A similar problem can be seen for most of the functions of this file.
https://github.com/jbx-protocol/juice-contracts-v2-code4rena/blob/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBTokenStore.sol#L115
Because the admin (controller) can burn a number of a specific token If the command to burn the token and the function to calculate the supply of the token are called and executed almost at the same time. Therefore, the amount declared in the TotalSupply variable will not be correct.
https://github.com/jbx-protocol/juice-contracts-v2-code4rena/blob/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBTokenStore.sol#L240
When the controller replaces an old token with a new one, it usually needs to work with the new token afterwards. Therefore, declaring the new token as return parameter seems more logical than returning the old token. Therefore, the output of this function should be changed to : (IJBToken _token)
https://github.com/jbx-protocol/juice-contracts-v2-code4rena/blob/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBTokenStore.sol#L266
If the declared address in (_newOwner) is wrong for any reason, there is no way to correct this mistake and the old tokens become useless. It is strongly recommended to use the two-step changeover method. In first step address of the new owner should be sent to the contract, and then the delegation of authority should be requested through the new address.
https://github.com/jbx-protocol/juice-contracts-v2-code4rena/blob/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBTokenStore.sol#L301
If there is a limit for total supply, this limit is not considered in adding new tokens. It means that it is not checked that the new mint will cause the new supply to exceed the total supply
https://github.com/jbx-protocol/juice-contracts-v2-code4rena/blob/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBSingleTokenPaymentTerminalStore.sol#L696
No modifier or input variable validation and checker is set for the input in the function. Any user can call and set it
Insecurity by compelexity:
This idea is implemented in a complex way .It is complicated to keep track of all the processes and workflows. This raises the possibility of hidden bugs. It is suggested to simplify part of the processes and workflow and data. Principle of Economy of Mechanism: “Keep the design as simple and small as possible” — Ensure that contracts and functions are not overly complex or large so as to reduce readability or maintainability. Complexity typically leads to insecurity