GalloDaSballo
commented
2 years ago Missing two step transfer of Guardian access role in line 60 to 63 of USDMPegRecovery.sol Disagree
Non-adherence to the checks-effects-interactions pattern. Appreciate the finding but the impl of those tokens is known and as such you can't trigger re-entrancy, in lack of POC I think this one is invalid
195
Missing Zero Address Check: Agree
All in all short and sweet, just missing some other findings
Missing two step transfer of Guardian access role in line 60 to 63 of USDMPegRecovery.sol
(https://github.com/code-423n4/2022-02-concur/blob/72b5216bfeaa7c52983060ebfc56e72e0aa8e3b0/contracts/USDMPegRecovery.sol#L60-L62)
Changing critical addresses in contracts should be a two-step process where the first transaction (from the old/current address) registers the new address (i.e. grants ownership) and the second transaction (from the new address) replaces the old address with the new one (i.e. claims ownership).
Non-adherence to the checks-effects-interactions pattern.
USDM balance in all the functions involving transfers should be updated before making the transfers to adhere to the checks-effects-interacts pattern/best practice and avoid any possible re-entrancy in the future i.e in this function. https://github.com/code-423n4/2022-02-concur/blob/72b5216bfeaa7c52983060ebfc56e72e0aa8e3b0/contracts/USDMPegRecovery.sol#L110-L117