Audittens - NGT minting is not possible via DssVest

[sherlock-admin2]

Audittens

Medium

NGT minting is not possible via DssVest

Summary

As stated in the README, rewards are going to be generated through a DssVestMintable, therefore it should have access to mint NGT tokens. However, no ngt.rely(vest) is being made in neither the VestInit, nor any other place in the scope.

Vulnerability Detail

As was stated in the ChainSecurity's MakerDAO Endgame Toolkit audit, item 6.2 "Vest Minting Not Possible":

DssVestMintable.pay() calls the NGT token's mint() function to generate tokens for the vesting. The function is guarded and can only be accessed by a ward. The DssVestMintable contract is never set as a ward of the NGT contract.

It's also stated in the audit that MakerDAO fixed the issue by adding the following line into the initialization script: RelyLike(ngt).rely(vest);. In practice, it was not added, and the script still lacks this line.

Impact

It is impossible for VestedRewardsDistribution to get rewards since DssVest won't be able to successfully execute the internal pay function. This leads to depositors of stakingRewards not getting their expected profit, which can be considered as a corresponding loss of funds for them.

Code Snippet

https://github.com/sherlock-audit/2024-06-makerdao-endgame/blob/main/endgame-toolkit/script/dependencies/VestInit.sol#L31

Tool Used

Manual Review

Recommendation

Add ngt as a parameter to VestInit.init, and add RelyLike(ngt).rely(vest); into VestInit.init.

[amusingaxl]

Please notice that the issue pointed was applicable to Version 1 of the report. The code was corrected then, however, as the repository evolved with newer requirements, so did our understanding of the ownership of the DssVestMintable contract. It is assumed now to be a fully set-up external contract.

Looking closely at the ChainSecurity Audit Report, you can check section 2.2.5 Changes in Version 3:

The rely() call was replaced by a check on wards() in a higher-level library:

https://github.com/makerdao/endgame-toolkit/blob/7a8665e976eb70dc19286814013c2b986e7b8816/script/dependencies/phase-0/FarmingInit.sol#L59

We suggest closing this as a non-issue.