NestedBuybacker sends NST to NestedReserve with no proper way to retrieve it.

[code423n4]

Handle

TomFrench

Vulnerability details

Impact

Co-mingling of DAO and user funds for no reason. Requirement to deploy a specialised factory to retrieve funds and store them somewhere more suitable.

Proof of Concept

When triggered the NestedBuybacker sends NST to the NestedReserve

https://github.com/code-423n4/2021-11-nested/blob/5d113967cdf7c9ee29802e1ecb176c656386fe9b/contracts/NestedBuybacker.sol#L37-L38

https://github.com/code-423n4/2021-11-nested/blob/5d113967cdf7c9ee29802e1ecb176c656386fe9b/contracts/NestedBuybacker.sol#L106-L113

This places this NST in the same contract as user owned NST with no proper bookkeeping (the amount of NST owned by the DAO needs to be calculated offchain). There's also no functions to retrieve this NST without impersonating a factory with a new contract specially deployed for the task.

https://github.com/code-423n4/2021-11-nested/blob/main/contracts/NestedReserve.sol

Recommended Mitigation Steps

Have a separate treasury contract to hold this NST which has been bought which allows safe withdrawal of this NST without the potential for accidentally stealing user funds.

[adrien-supizet]

The nstReserve mentioned here https://github.com/code-423n4/2021-11-nested/blob/5d113967cdf7c9ee29802e1ecb176c656386fe9b/contracts/NestedBuybacker.sol#L106-L113
is an EOA and not the NestedReserve contract used to store funds.

The wording can be confusing and can be adapted, but this is not an issue.

[alcueca]

Downgraded to code clarity issue.