search

code-423n4 / 2021-09-yaxis-findings

0 stars 0 forks source link

manager.allowedVaults check missing for add/remove strategy #50

Open code423n4 opened 3 years ago

code423n4 commented 3 years ago

Handle

0xRajeev

Vulnerability details

Impact

The manager.allowedVaults check is missing for add/remove strategy like how it is used in reorderStrategies(). This will allow a strategist to accidentally/maliciously add/remove strategies on unauthorized vaults.

Given the critical access control that is missing on vaults here, this is classified as medium severity.

Proof of Concept

https://github.com/code-423n4/2021-09-yaxis/blob/cf7d9448e70b5c1163a1773adb4709d9d6ad6c99/contracts/v3/controllers/Controller.sol#L101-L130

https://github.com/code-423n4/2021-09-yaxis/blob/cf7d9448e70b5c1163a1773adb4709d9d6ad6c99/contracts/v3/controllers/Controller.sol#L172-L207

https://github.com/code-423n4/2021-09-yaxis/blob/cf7d9448e70b5c1163a1773adb4709d9d6ad6c99/contracts/v3/controllers/Controller.sol#L224

https://github.com/code-423n4/2021-09-yaxis/blob/cf7d9448e70b5c1163a1773adb4709d9d6ad6c99/contracts/v3/Manager.sol#L210-L221

Tools Used

Manual Analysis

Recommended Mitigation Steps

Add manager.allowedVaults check in addStrategy() and removeStrategy()

uN2RVw5q commented 2 years ago

Implemented in https://github.com/code-423n4/2021-09-yaxis/pull/36

GalloDaSballo commented 2 years ago

Sponsor has acknowledged and mitigated by adding further access control checks