code-423n4 / 2023-01-astaria-findings

5 stars 2 forks source link

Purchaser of a lien token may not receive payments #619

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-01-astaria/blob/main/src/LienToken.sol#L812-L849 https://github.com/code-423n4/2023-01-astaria/blob/main/src/LienToken.sol#L900-L909 https://github.com/code-423n4/2023-01-astaria/blob/main/src/LienToken.sol#L911-L919

Vulnerability details

Impact

A purchaser who buys out an existing lien via buyoutLien() will not receive future payments made to that lien holder if the seller had changed the lien payee via setPayee() and if they do not change it themselves after buying.

buyoutLien() does not reset lienData[lienId].payee to either 0 or to the new owner. While the ownership is transferred, the payments made in _payment() get their payee via getPayee() which returns the owner only if the lienData[lienId].payee is set to the zero address but returns lienData[lienId].payee otherwise. This will still have the value set by the previous owner who will continue to receive the payments.

The purchaser who buys out an existing lien via buyoutLien() will not receive future payments if the previous owner had set the payee but they do not change it via setPayee() themselves after buying. Given that setPayee() is considered optional (nothing specified in spec/docs/comments) and this reset is not part of the default flow, this will lead to a loss of purchaser's anticipated payments until they realize and reset the lien payee.

Proof of Concept

A malicious lien seller can use setPayee() to set the payee to their address of choice and continue to receive payments, even after selling, if the buyer does not realize that they have to change the payee address to themselves after buying.

Tools Used

Manual Review

Recommended Mitigation Steps

buyoutLien() should reset lienData[lienId].payee to either the zero address or to the new owner.

c4-sponsor commented 1 year ago

SantiagoGregory marked the issue as disagree with severity

c4-sponsor commented 1 year ago

SantiagoGregory marked the issue as sponsor disputed

SantiagoGregory commented 1 year ago

Sorry, the "disagree with severity" was an accident. However, _setPayee() is internal and only used to set a LienToken payee to a WithdrawProxy if the lien expires near an epoch boundary. A lien cannot be bought out if the lien has been liquidated.

c4-judge commented 1 year ago

Picodes marked the issue as unsatisfactory: Invalid

Picodes commented 1 year ago

Indeed _setPayee is internal and used only in case of liquidation.