The updateAdminsContract(address) function in RandomizerNXT contract doesn't check if the proposed new admin address implements isAdminContract()

[c4-submissions]

Lines of code

https://github.com/code-423n4/2023-10-nextgen/blob/8b518196629faa37eae39736837b24926fd3c07c/hardhat/smart-contracts/RandomizerNXT.sol#L45-L46

Vulnerability details

Bug Description

RandomizerRNG and RandomizerVRF contracts execute a check in updateAdminContract(address) function to make sure the newly proposed admin address corresponds to a contract that implements isAdminContract(), which only returns true. As updateAdminContract() uses a one step procedure to update the value of INextGenAdmins private adminsContract variable, this check is important to make sure the address passed to updateAdminContract() indeed corresponds to an implementation of NextGenAdmins contract.

Impact

Here is the check :

require(INextGenAdmins(_newadminsContract).isAdminContract() == true, "Contract is not Admin");

This line isn't present in RandomizerNXT. Therefore, there is no check on the address passed and what contract it corresponds to. Moreover, as all update functions in RandomizerNXT use FunctionAdminRequired modifier, if a wrong address is passed, they will all fail as the modifier execution will revert. In that situation, it will be necessary to redeploy RandomizerNXT contract.

Tools Used

Manual

Recommended Mitigation Steps

Add the same check as in other Randomizer contracts to make sure the new adminsContract variable will be an adminsContract instance and will therefore be callable :

require(INextGenAdmins(_newadminsContract).isAdminContract() == true, "Contract is not Admin");

Assessed type

Access Control

[c4-pre-sort]

141345 marked the issue as duplicate of #478

[c4-judge]

alex-ppg marked the issue as not a duplicate

[alex-ppg]

Similarly to #123, the isAdminContract is a rudimentary safety check rather than a security check. As such, I deem this submission to be a QA (NC) and thus of overinflated severity.

[c4-judge]

alex-ppg marked the issue as unsatisfactory:
Overinflated severity