``destination`` address can't actually call ``transferOut()`` of ``UserEscrow.sol``

[c4-submissions]

Lines of code

https://github.com/code-423n4/2023-09-centrifuge/blob/512e7a71ebd9ae76384f837204216f26380c9f91/src/UserEscrow.sol#L36-L50

Vulnerability details

Impact

The @dev comment of transferOut function suggests transferOut can only be initiated by the destination address or an authorized admin but auth modifier prevents that. It only allows selected wards but making every destination address a ward is not feasible as ward in auth modifier is a trusted role and has access to all the major functionality in the protocol.

It bricks a major functionality. The tokens can only be transferred out by authorized ward. Thus, the whole design of transferOut is faulty.

Proof of Concept

Not required

Tools Used

Manual Analysis

Recommended Mitigation Steps

This can be mitigated by only allowing authorized ward to call transferOut function.

Assessed type

Other

[c4-pre-sort]

raymondfam marked the issue as sufficient quality report

[c4-pre-sort]

raymondfam marked the issue as duplicate of #217

[c4-judge]

gzeon-c4 marked the issue as not a duplicate

[c4-judge]

gzeon-c4 marked the issue as primary issue

[c4-judge]

gzeon-c4 marked the issue as unsatisfactory: Invalid

[c4-judge]

gzeon-c4 removed the grade

[gzeon-c4]

This is called from InvestmentManager which is a ward of UserEscrow, however it still seems to require auth on the IM to call anything to lead to transferOut. Looks like an ambiguous comment transferOut can only be initiated by the destination address or an authorized admin.

[hieronx]

Yeah the implementation is correct, but the comment is wrong/misleading. It should be transferOut can only be initiated by an authorized admin, to a destination set on transferIn.

[c4-judge]

gzeon-c4 changed the severity to QA (Quality Assurance)